FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[release/9.0] Fix length check for Convert.TryToHexString{Lower} by github-actions[bot] · Pull Request #110228 · dotnet/runtime · GitHub

  1. dotnet
  2. runtime

Repository navigation

[release/9.0] Fix length check for Convert.TryToHexString{Lower} - #110228

Merged
rbhanda merged 3 commits into
release/9.0from
backport/pr-109891-to-release/9.0-staging
Dec 3, 2024
ConversationCommits (3)ChecksFiles changed
Merged

[release/9.0] Fix length check for Convert.TryToHexString{Lower}#110228
rbhanda merged 3 commits into
release/9.0from
backport/pr-109891-to-release/9.0-staging

Conversation

github-actions Bot commented Nov 27, 2024 •
edited by adamsitnik
Loading

Copy link
Copy Markdown
Contributor

Backport of #109891 to release/9.0-staging

/cc @adamsitnik @universorum

Customer Impact

  • Customer reported
  • Found internally

The customers can't use the new Convert.TryToHexString{Lower} methods that got added in .NET 9, as due to an invalid condition (> swapped with <), we were rejecting buffers that were large enough to be able to store the results.

Regression

  • Yes
  • No

No, both overloads got added in .NET 9.

Testing

The issue was missed because the overload seemed simple enough to not require any new tests.

The fix contains a nice set of unit tests that exercise both happy and unhappy code paths.

Risk

I can't see any risks, it's a very, very simple fix.

ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 27, 2024
adamsitnik added area-System.Runtime and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 27, 2024

dotnet-policy-service Bot commented Nov 27, 2024

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

adamsitnik self-assigned this Nov 27, 2024
adamsitnik added the Servicing-consider Issue for next servicing release review label Nov 27, 2024
adamsitnik added this to the 9.0.x milestone Nov 27, 2024
build-analysis Bot mentioned this pull request Nov 27, 2024
Open
3 tasks
jeffhandley added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 27, 2024
GrabYourPitchforks changed the base branch from release/9.0-staging to release/9.0 December 2, 2024 22:35
GrabYourPitchforks changed the base branch from release/9.0 to release/9.0-staging December 2, 2024 22:36
GrabYourPitchforks changed the base branch from release/9.0-staging to release/9.0 December 2, 2024 22:40
GrabYourPitchforks force-pushed the backport/pr-109891-to-release/9.0-staging branch from 84a732f to 3ba2b3a Compare December 2, 2024 22:41
carlossanlop approved these changes Dec 2, 2024
View reviewed changes

carlossanlop left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yay, 2 files once again after the force push. I assume I can merge it now @GrabYourPitchforks ?

Edit: Let's wait for the CI to finish, there's still time.

GrabYourPitchforks commented Dec 2, 2024

Copy link
Copy Markdown
Member

Rebased the current PR on release/9.0 per discussion with servicing team.

GrabYourPitchforks commented Dec 2, 2024

Copy link
Copy Markdown
Member

@carlossanlop Let's wait for CI to finish first. (Or at least to get most of the way through so we have good confidence we didn't break anything.)

GrabYourPitchforks changed the title [release/9.0-staging] Fix length check for Convert.TryToHexString{Lower} [release/9.0] Fix length check for Convert.TryToHexString{Lower} Dec 2, 2024
This was referenced Dec 3, 2024
Closed
Closed

GrabYourPitchforks commented Dec 3, 2024

Copy link
Copy Markdown
Member

@carlossanlop CI failures are unrelated. Feel free to merge!

rbhanda merged commit 9da8c6a into release/9.0 Dec 3, 2024
carlossanlop deleted the backport/pr-109891-to-release/9.0-staging branch December 3, 2024 19:01
github-actions Bot locked and limited conversation to collaborators Jan 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

jeffhandley jeffhandley approved these changes
GrabYourPitchforks GrabYourPitchforks approved these changes
+1 more reviewer
carlossanlop carlossanlop approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

adamsitnik

Labels

area-System.Runtime Servicing-approved Approved for servicing release

Projects

None yet

Milestone

9.0.x

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL