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

Update System.Text.Json from 8.0.4 to 8.0.5 #6101

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Conversation

dtivel
Copy link
Contributor

@dtivel dtivel commented Oct 15, 2024

Bug

Fixes: NuGet/Home#13857

Description

This change updates System.Text.Json from 8.0.4 (with security vulnerability) to 8.0.5 (latest release).

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests N/A
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc. N/A

@dtivel dtivel requested a review from a team as a code owner October 15, 2024 17:28
@aortiz-msft aortiz-msft self-requested a review October 15, 2024 17:52
@aortiz-msft
Copy link
Contributor

@marcpopMSFT - This change is going into 17.13. Is this good to merge now?

@jeffkl
Copy link
Contributor

jeffkl commented Oct 15, 2024

@Nigusu-Allehu
Copy link
Contributor

MSBuild has been updated, its probably safe: https://github.com/dotnet/msbuild/pull/10776/files#diff-af84f50fd5aa2e8bfc3479cb6ddf7882bc084ce1af01de6dd49ff93aa2d1718cR138

Should we not wait for VS as well?

Copy link
Contributor

@donnie-msft donnie-msft left a comment

Choose a reason for hiding this comment

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

Thanks, Damon! I saw this error with local builds. Not sure why CI doesn't fail because of it.

Copy link
Member

@zivkan zivkan left a comment

Choose a reason for hiding this comment

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

We need confirmation this isn't going to cause problems, before we merge. See also:

I still need to port a more advanced version of this PR to the dev branch:

See the PR description of this last PR for more details about why we can't upgrade some packages (like this one) until our downstream components are ready to take the change

@zivkan
Copy link
Member

zivkan commented Oct 15, 2024

Apex tests were useful in this case. We can see that VS main branch is still shipping an older version of System.Text.Json, so NuGet in VS is failing because of an assembly load failure.

@dtivel
Copy link
Contributor Author

dtivel commented Oct 15, 2024

Apex tests were useful in this case. We can see that VS main branch is still shipping an older version of System.Text.Json, so NuGet in VS is failing because of an assembly load failure.

Yep, super useful! @zivkan, should I close this PR?

@marcpopMSFT
Copy link

@rainersigwald has been traying to coordinate the update of VS on STJ. I believe it's too late for 17.11, he plans to talk to the QB for 17.12, and we definitely want it for 17.13 but it has to be coordinated with MS Build AND VS. Not sure if the VS work for 17.13 is done yet.

@rainersigwald
Copy link
Contributor

We MUST NOT reference STJ 8.0.5 for NuGet 6.12/SDK 9.0.100/VS 17.12. For 17.13/9.0.200/6.13 you will be clear to reference 8.0.5 as soon as MSBuild can manage to insert (any minute now; currently that's https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/585290). So I wouldn't close, but maybe pause for a day or two?

Copy link
Contributor

@Nigusu-Allehu Nigusu-Allehu left a comment

Choose a reason for hiding this comment

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

I think we are good to go now. The MSBuild insertion updating STJ in VS to 8.05 is through.

Copy link
Contributor

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Yup, :shipit:

cc @YuliiaKovalova -- yell at NuGet if we have to revert STJ in VS for some reason.

@zivkan zivkan merged commit fbb1d63 into dev Oct 16, 2024
29 of 31 checks passed
@zivkan zivkan deleted the dev-dtivel-system-text-json branch October 16, 2024 20:10
zivkan added a commit to zivkan/NuGet.Client that referenced this pull request Oct 28, 2024
zivkan added a commit that referenced this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.Text.Json 8.0.4 has security vulnerability
8 participants