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.

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

System.Net.Http does not behave as an OOB package when used with System.Net.Http.Formatting #17786

Closed
neoaisac opened this issue Jul 7, 2016 · 15 comments
Assignees
Labels
area-System.Net packaging Related to packaging
Milestone

Comments

@neoaisac
Copy link

neoaisac commented Jul 7, 2016

This issue has been created to separate it from the conversation in issue #17770.

As explained there, when using System.Net.Http v4.0.0.0 as distributed by the .NET Framework 4.6.1, everything works correctly.

When updating the reference to the NuGet-distributed package for System.Net.Http v4.1.0.0 (here: https://www.nuget.org/packages/System.Net.Http/) the code fails at runtime whith the below exception:

[VerificationException: Method System.Net.Http.CloneableExtensions.Clone: type argument 'System.Net.Http.Headers.MediaTypeHeaderValue' violates the constraint of type parameter 'T'.]
System.Net.Http.Formatting.MediaTypeConstants.get_ApplicationJsonMediaType() +0
System.Net.Http.Formatting.JsonMediaTypeFormatter..ctor() +79
System.Net.Http.Formatting.MediaTypeFormatterCollection.CreateDefaultFormatters() +49
System.Web.Http.HttpConfiguration..ctor(HttpRouteCollection routes)
...

When reverting to the out-of-the box v.4.0.0.0 version it works as expected without problems.

Therefore, version 4.1.0.0 is not an OOB package as it breaks the functionality of other packages that relate to this one.

@neoaisac
Copy link
Author

neoaisac commented Jul 7, 2016

To expand on my above comment, the problem seems to be with the type System.ICloneable (source here: http://aspnetwebstack.codeplex.com/sourcecontrol/latest#src/System.Net.Http.Formatting/CloneableExtensions.cs).

It seems that corefx's implementation uses a custom-made ICloneable interface, therefore not meeting the type argument requirements of that class. (See corefx's custom-made internal ICloneable here: https://github.com/dotnet/corefx/blob/d0dc5fc099946adc1035b34a8b1f6042eddb0c75/src/System.Net.Http/src/Internal/ICloneable.cs).

I have tried to reference this one, though and I can't see it being used, so I may be wrong. But the error persists so it must be something related to this interface and how corefx uses it in this library.

@davidsh
Copy link
Contributor

davidsh commented Jul 7, 2016

cc: @ericstj @stephentoub

@stephentoub
Copy link
Member

cc: @danmosemsft, @KrzysztofCwalina
This appears to be due to the lack of ICloneable in .NET Core.

@neoaisac
Copy link
Author

neoaisac commented Jul 7, 2016

I would think so too, but then this disrupts the whole definition of OOB package, then.

@ericstj
Copy link
Member

ericstj commented Jul 7, 2016

I wasn't aware that any type-subsetting was done with System.Net.Http.dll. Indeed if that exposes different surface area than the reference assembly in the package, and it has the same name, that's a breaking change that needs to be fixed.

@ericstj
Copy link
Member

ericstj commented Jul 7, 2016

Indeed if I diff the targeting pack with the nupkg reference assembly I see this difference: https://gist.github.com/ericstj/5db3de3de78c249b1138601eb9a79423#systemnethttpheaders

stephentoub referenced this issue in stephentoub/corefx Jul 11, 2016
We've got a bunch of types outside of mscorlib that implement this interface in desktop, and the lack of it is causing issues, e.g.  https://github.com/dotnet/corefx/issues/9884.  This commit adds the interface to the contract; when we subsequently update the packages, then we can start implementing this in types outside of System.Runtime (since System.Runtime is mostly a facade on System.Private.Corelib, the types it "contains" already implement the interface).
@JohnGalt1717
Copy link

Just a heads up but this isn't the only issue with cross compatibility. System.IdentyModel.Tokens.Jwt is also borked completely and breaks existing .NET 4.6 solutions that worked with the old version and you're pushing this broken stuff on nuget.

@davidsh
Copy link
Contributor

davidsh commented Jul 14, 2016

@ericstj Should I assign this issue to you since it has to do with packaging, etc.? Not sure how to solve this OOB issue.

@ryanrust
Copy link

This issue is definitely causing problems for users of packages that have added targetFramework usage of .NETStandard to their nuspec files. We make use of AutoMapper extensively in our project and and are running into this issue with the latest update.

@ericstj
Copy link
Member

ericstj commented Jul 14, 2016

This isn't about packaging, as i pointed out the contract is missing ICloneable. That needs to be added back, at least to the desktop implementation you're providing out of band.

@danmosemsft and @stephentoub are adding this back to contract surface area but that won't fix this case since that will have to be in a new NETStandard version that old desktop profiles won't support.

The best fix here is for you to cross compile System.Net.Http as a desktop assembly, built against the desktop targeting pack, and make sure you implement ICloneable in that implementation.

@davidsh
Copy link
Contributor

davidsh commented Aug 17, 2016

This should be fixed now with dotnet/corefx#10716.

@davidsh davidsh closed this as completed Aug 17, 2016
@petersondrew
Copy link

Is there a timeline on when this fix will be pushed to Nuget?

@davidsh
Copy link
Contributor

davidsh commented Sep 15, 2016

This will be part of the .NET Core 1.1 release. I believe the timing is within a few months. There will be a preview pushed to NuGet before the actual RTM (stable packages) pushed to NuGet.

cc: @richlander, @leecow can comment on release timeframe for .NET Core 1.1

@leecow
Copy link
Member

leecow commented Sep 15, 2016

We're shooting to have the 1.1 Preview available in October. Watch the usual channels (@dotnet on Twitter and https://blogs.msdn.microsoft.com/dotnet/) for the official announcement and details.

@jahmai-ca
Copy link

Couldn't this be fixed in isolation of 1.1? This is affecting us and it seems like it'd affect a good chunk of people referencing NETStandard and System.Net.Http.Formatters together, not great to leave broken for months.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net packaging Related to packaging
Projects
None yet
Development

No branches or pull requests

10 participants