Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix xml serializer encoding #3891

Closed
wants to merge 24 commits into from
Closed

Fix xml serializer encoding #3891

wants to merge 24 commits into from

Conversation

maheshnlrb
Copy link
Contributor

For #3870

@dnfclas
Copy link

dnfclas commented Sep 9, 2019 •
edited
Loading

CLA assistant check
All CLA requirements met.

HongGit and others added 7 commits September 9, 2019 12:12
StephenBonikowsky
StephenBonikowsky reviewed Sep 11, 2019
View reviewed changes
...m.Private.ServiceModel/src/System/ServiceModel/Dispatcher/XmlSerializerOperationFormatter.cs Outdated
@@ -83,7 +90,8 @@ protected override void AddHeadersToMessage(Message message, MessageDescription
MemoryStream memoryStream = new MemoryStream();
XmlDictionaryWriter bufferWriter = XmlDictionaryWriter.CreateTextWriter(memoryStream);
bufferWriter.WriteStartElement("root");
serializer.Serialize(bufferWriter, headerValues, null);
// serializer.Serialize(bufferWriter, headerValues, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed entirely.

@StephenBonikowsky
Copy link
Member

Just a couple initial nits, @mconnew or I will do a little deeper review in the next day or so.
Thanks for doing this @maheshnlrb !

@StephenBonikowsky
Merge pull request dotnet#3899 from dotnet/hongli/3864
3a91b5d 
[P2] Support System.ServiceModel.Security.Tokens.SecureConversationSe…
@mconnew
Copy link
Member

mconnew commented Sep 11, 2019

@maheshnlrb, I've verified you have fixed all the places where code is missing.
@StephenBonikowsky, we should leave the issue open as we should have scenario tests to validate this functionality.

@mconnew
Copy link
Member

mconnew commented Sep 11, 2019

@maheshnlrb, it looks like you need to rebase your branch as github has found some conflicts and can't automatically merge your fixes.

@maheshnlrb
Copy link
Contributor Author

@maheshnlrb, it looks like you need to rebase your branch as github has found some conflicts and can't automatically merge your fixes.

@mconnew, I merged wcf/mater to my branch, but I am getting lot of errors while package restore. What do I need to do to fix this?
'C:\Test\DotNetCore\wcf\src\System.Private.ServiceModel\tests\Common\Unit\UnitTests.Common.csproj : error NU1102: Unable to find package Microsoft.NET.Test.Sdk with version (>= 16.1.1) [C:\Test\DotNetCore\wcf\System.ServiceModel.sln]
C:\Test\DotNetCore\wcf\src\System.Private.ServiceModel\tests\Common\Unit\UnitTests.Common.csproj : error NU1102: - Found 24 version(s) in dotnet-core [ Nearest version: 15.5.0-preview-20170810-02 ] [C:\Test\DotNetCore\wcf\System.ServiceModel.sln]
C:\Test\DotNetCore\wcf\src\System.Private.ServiceModel\tests\Common\Infrastructure\Infrastructure.Common.csproj : error NU1102: Unable to find package Microsoft.NET.Test.Sdk with version (>= 16.1.1) [C:\Test\DotNetCore\wcf\System.ServiceModel.sln]
C:\Test\DotNetCore\wcf\src\System.Private.ServiceModel\tests\Common\Infrastructure\Infrastructure.Common.csproj : error NU1102: - Found 24 version(s) in dotnet-core [ Nearest version: 15.5.0-preview-20170810-02 ] [C:\Test\DotNetCore\wcf\System.ServiceModel.sln]
C:\Test\DotNetCore\wcf\src\System.Private.ServiceModel\tests\Common\Infrastructure\Infrastructure.Common.csproj : error NU1102: Unable to find package xunit.extensibility.core with version (>= 2.4.1) [C:\Test\DotNetCore\wcf\System.ServiceModel.sln]
C:\Test\DotNetCore\wcf\src\System.Private.ServiceModel\tests\Common\Infrastructure\Infrastructure.Common.csproj : error NU1102: - Found 1 version(s) in dotnet-core [ Nearest version: 2.1.0 ] [C:\Test\DotNetCore\wcf\System.ServiceModel.sln]
C:\Test\DotNetCore\wcf\src\System.Private.ServiceModel\tests\Common\Infrastructure\Infrastructure.Common.csproj : error NU1102: Unable to find package xunit.extensibility.execution with version (>= 2.4.1) [C:\Test\DotNetCore\wcf\System.ServiceModel.sln]
C:\Test\DotNetCore\wcf\src\System.Private.ServiceModel\tests\Common\Infrastructure\Infrastructure.Common.csproj : error NU1102: - Found 1 version(s) in dotnet-core [ Nearest version: 2.1.0 ] [C:\Test\DotNetCore\wcf\System.ServiceModel.sln]'

@mconnew
Copy link
Member

mconnew commented Sep 12, 2019

@maheshnlrb, I don't know what's going on. Did you try cleaning your workspace? Also I always rebase instead of merging as I've had better luck with that. But once you've merged, rebasing afterwards can mess things up.

HongGit and others added 6 commits September 11, 2019 21:21
@HongGit @mconnew
fixing a build error
4d7e8de
@StephenBonikowsky @mconnew
Add S.IM.T.SymmetricSecurityKey to the public contract.
3ca5caa 
* Also had to make the class public in the implementation.
@dotnet-maestro
[master] Update dependencies from dotnet/arcade (dotnet#3894)
5bcefd7 
* Update dependencies from https://github.com/dotnet/arcade build 20190909.10

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19459.10
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19459.10
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19459.10
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19459.10
- Microsoft.DotNet.SignTool - 1.0.0-beta.19459.10

* Update dependencies from https://github.com/dotnet/arcade build 20190910.3

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19460.3
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19460.3
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19460.3
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19460.3
- Microsoft.DotNet.SignTool - 1.0.0-beta.19460.3

* Update dependencies from https://github.com/dotnet/arcade build 20190911.7

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19461.7
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19461.7
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19461.7
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19461.7
- Microsoft.DotNet.SignTool - 1.0.0-beta.19461.7
@maheshnlrb @MaheshVcsoft
Fixed XML Serializer to include encoding if required.
1527f60
@MaheshVcsoft
Formatting fixes
628ebb6
@MaheshVcsoft
Merge branch 'FixXmlSerializerEncoding' of https://github.com/maheshn…
62d3d72 
…lrb/wcf into FixXmlSerializerEncoding

# Conflicts:
#	src/System.Private.ServiceModel/src/System/ServiceModel/Dispatcher/XmlSerializerOperationFormatter.cs
StephenBonikowsky
StephenBonikowsky reviewed Oct 4, 2019
View reviewed changes
...m.Private.ServiceModel/src/System/ServiceModel/Dispatcher/XmlSerializerOperationFormatter.cs
@@ -236,7 +243,7 @@ protected override void GetHeadersFromMessage(Message message, MessageDescriptio
if (!bufferReader.IsEmptyElement)
{
bufferReader.ReadStartElement();
object[] headerValues = (object[])serializer.Deserialize(bufferReader);
object[] headerValues = (object[])serializer.Deserialize(bufferReader, isEncoded ? GetEncoding(message.Version.Envelope) : null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as other comment, should be _isEncoded

@StephenBonikowsky
Copy link
Member

The build errors are because of the name change from isEncoded to _isEncoded

Once that is fixed we just need to clean-up the commit history mess.

@StephenBonikowsky StephenBonikowsky mentioned this pull request Oct 28, 2019
Closed
2 tasks
StephenBonikowsky added a commit to StephenBonikowsky/wcf that referenced this pull request Oct 29, 2019
@StephenBonikowsky
Add "encodingStyle" attribute to generated SOAP message.
8f35af5 
* This fix was initially produced by @maheshnlrb with PR dotnet#3891 but the commit history got sufficiently scrambled that is was easier to reproduce his changes in a clean up-to-date branch.
@StephenBonikowsky StephenBonikowsky mentioned this pull request Oct 29, 2019
Merged
@StephenBonikowsky
Copy link
Member

I created a new PR #4001 with a clean commit history.
Thanks @maheshnlrb for doing the work.

StephenBonikowsky added a commit to StephenBonikowsky/wcf that referenced this pull request Nov 4, 2019
@StephenBonikowsky
Add "encodingStyle" attribute to generated SOAP message.
759d4f1 
* This fix was initially produced by @maheshnlrb with PR dotnet#3891 but the commit history got sufficiently scrambled that is was easier to reproduce his changes in a clean up-to-date branch.
StephenBonikowsky added a commit to StephenBonikowsky/wcf that referenced this pull request Nov 4, 2019
@StephenBonikowsky
Add "encodingStyle" attribute to generated SOAP message.
e865c42 
* This fix was initially produced by @maheshnlrb with PR dotnet#3891 but the commit history got sufficiently scrambled that is was easier to reproduce his changes in a clean up-to-date branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mconnew mconnew mconnew left review comments

@StephenBonikowsky StephenBonikowsky StephenBonikowsky left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@maheshnlrb @dnfclas @StephenBonikowsky @mconnew @HongGit @MaheshVcsoft