Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fix Wave Analysis issues#3403

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
cheenamalhotra merged 3 commits intomainfromdev/paul/wave-analysis
Jun 11, 2025
Merged

Conversation

paulmedynski
Copy link
Contributor

Description

  • Updated .NET to explicitly use System.Text.Json 8.0.5 or 9.0.5 to avoid transitive vulnerabilities.
  • Updated test tools to use Microsoft.SqlServer.SqlManagementObjects 172.76.0 to avoid transitive vulnerabilities.
  • Sorted PackageReference entries alphabetically by package name.
  • Updated .NET Framework project to build on Linux.

Issues

ADO 37675

Testing

  • Checked that all affected projects build.
  • CI will find any regressions.

Frulfump reacted with thumbs up emoji
@paulmedynskipaulmedynski added this to the6.1-preview2 milestoneJun 9, 2025
@CopilotCopilotAI review requested due to automatic review settingsJune 9, 2025 09:37
@paulmedynskipaulmedynski requested a review froma team as acode ownerJune 9, 2025 09:37
Copy link
Contributor

@CopilotCopilotAI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR pins explicit versions for System.Text.Json and other dependencies to address transitive vulnerabilities, reorders and updates package references, and adjusts project files to improve Linux build support.

  • Add System.Text.Json dependencies and bump SqlManagementObjects to mitigate vulnerabilities
  • Sort and standardize<PackageReference> entries and version props
  • Update netfx csproj (SDK name, code analysis rule, remove COM reference, adjust compile includes)

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
FileDescription
tools/specs/Microsoft.Data.SqlClient.nuspecAdd System.Text.Json dependency for net7.0, net9.0, netstandard2.0
tools/props/VersionsNet9OrLater.propsIntroduceSystemTextJsonVersion and fix property ordering
tools/props/Versions.propsRelocateSystemTextJsonVersion and bump SqlManagementObjects
src/Microsoft.Data.SqlClient/netfx/...csprojChange SDK, update code analysis rule condition, rename files, remove COM reference, reorder packages
src/Microsoft.Data.SqlClient/netcore/...csprojRenameSqlMetadataFactory toSqlMetaDataFactory, reorder packages
src/Microsoft.Data.SqlClient/netcore/ref/...csprojReorder package references to match new standard
Comments suppressed due to low confidence (2)

src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj:951

  • Verify that the source file and its class declaration have been renamed to "DBConnectionString.cs" (uppercase 'DB') on disk; otherwise the case-sensitive Linux build may fail due to a mismatch.
<Compile Include="Microsoft\Data\Common\DBConnectionString.cs" />

src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj:663

  • Ensure the file and class name have been updated to "SqlMetaDataFactory.cs" (capital 'D') so the project reference matches the actual file on a case-sensitive filesystem.
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlMetaDataFactory.cs">

Copy link
ContributorAuthor

@paulmedynskipaulmedynski left a comment

Choose a reason for hiding this comment

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

Leaving some notes for reviewers.

@codecovCodecov
Copy link

codecovbot commentedJun 9, 2025•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.34%. Comparing base(e037c8a) to head(e82dd8b).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@##             main    #3403   +/-   ##=======================================  Coverage   65.34%   65.34%           =======================================  Files         301      301             Lines       65625    65625           =======================================+ Hits        42880    42882    +2+ Misses      22745    22743    -2
FlagCoverage Δ
addons92.58% <ø> (ø)
netcore68.57% <ø> (+<0.01%)⬆️
netfx66.77% <ø> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown.Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cheenamalhotra
Copy link
Member

I have this addressed in#3333

- Updated .NET to explicitly use System.Text.Json 8.0.5 or 9.0.5 to avoid transitive vulnerabilities.- Sorted PackageReference entries alphabetically by package name.- Updated test tools to use Microsoft.SqlServer.SqlManagementObjects 172.76.0 to avoid transitive vulnerabilities.- Updated .NET Framework project to build on Linux.
@paulmedynski
Copy link
ContributorAuthor

Merging in#3333 was a mess, so I had to rebase instead - sorry! The diffs are pretty small now though.

ErikEJ reacted with thumbs up emoji

@paulmedynskipaulmedynski requested a review froma teamJune 10, 2025 10:54
@ErikEJ
Copy link
Contributor

LGTM

- Added System.Text.Json 9.0.5 to .NET 9.0.- Fixed System.Formats.Asn1 transitive vulnerability.
- Removed unnecessary PackageReference Version attributes.
@cheenamalhotracheenamalhotra merged commit260940b intomainJun 11, 2025
251 checks passed
@cheenamalhotracheenamalhotra deleted the dev/paul/wave-analysis branchJune 11, 2025 20:39
<PackageReference Include="Microsoft.Bcl.Cryptography" />

<!-- Transitive dependencies that would otherwise bring in older, vulnerable versions. -->
<PackageReference Include="System.Text.Json" />

Choose a reason for hiding this comment

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

What references are pulling in the vulnerable version? Might make sense to add a comment so it will be easier to check when this can be removed.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ErikEJErikEJErikEJ left review comments

@apoorvdeshmukhapoorvdeshmukhapoorvdeshmukh left review comments

@FrulfumpFrulfumpFrulfump left review comments

Copilot code reviewCopilotCopilot left review comments

@benrr101benrr101benrr101 approved these changes

@cheenamalhotracheenamalhotracheenamalhotra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.1-preview2
Development

Successfully merging this pull request may close these issues.

6 participants
@paulmedynski@cheenamalhotra@ErikEJ@benrr101@apoorvdeshmukh@Frulfump
[8]ページ先頭
©2009-2025 Movatter.jp