-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
lower the baseline version for system.text.encodings.web #49671
Conversation
Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer Issue Details5.0.1 package version of system.text.encodings.web contains an updated assembly version (5.0.0.1). The shared framework contains 5.0.0.0 which leads to version conflicts.
|
Thank you for taking care of this Anirudh! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't do this without rolling back the version change and rolling back the version change is breaking since it's been publicly exposed.
Everything in the repo is building against the new version.
Perhaps you can keep the runtime version at 5.0.0.1 and only roll back the reference assembly version. The reference assembly should be used in this repo when building so this will prevent the newer version from leaking to other packages in this repo.
And by doing that, aspnetcore's usage of this package would not hit the AutoUnification issue in RAR anymore. I'm in favor of that. |
src/libraries/System.Text.Encodings.Web/ref/System.Text.Encodings.Web.csproj
Outdated
Show resolved
Hide resolved
@@ -6455,7 +6455,7 @@ | |||
"5.0.0", | |||
"5.0.1" | |||
], | |||
"BaselineVersion": "5.0.1", | |||
"BaselineVersion": "5.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious if we want to do this or not.
This would mean that JSON wouldn't bring the security fix made to STEW. What's your thinking @GrabYourPitchforks?
On the plus side, it would mean that ASP.NET doesn't see that new version from STEW (though they might be able to intentionally downgrade). Is it possible to have ASP.NET test with and without the baseline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't sound right to downgrade the baseline and miss the security fix when STEW is transitively referenced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed with ASP.NET folks over teams, let's undo the change to the baseline: keep it at 5.0.1 here. They'll make some customizations to their build to avoid consuming the versioned S.T.E.W.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"BaselineVersion": "5.0.0", | |
"BaselineVersion": "5.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would mean that JSON wouldn't bring the security fix made to STEW.
My main concern would be somebody adding a package ref to System.Text.Json, then to S.T.E.W, and as a result missing the fix because I believe NuGet will pull in the existing transitive package reference.
Co-authored-by: Eric StJohn <[email protected]>
Merging this one to unblock the builds, tested locally the assembly versions. 5.0.0.0 applies to the netstandard2.0 and net5.0 reference assemblies |
Restore |
5.0.1 package version of system.text.encodings.web contains an updated assembly version (5.0.0.1). The shared framework contains 5.0.0.0 which leads to version conflicts.