KEMBAR78
Add Base64.IsValid and allow Base64.DecodeXx methods to skip whitespace by stephentoub · Pull Request #85938 · dotnet/runtime · GitHub
Skip to content

Add Base64.IsValid and allow Base64.DecodeXx methods to skip whitespace #85938

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 9, 2023
Merged

Add Base64.IsValid and allow Base64.DecodeXx methods to skip whitespace #85938

merged 3 commits into from
May 9, 2023

Conversation

stephentoub
Copy link
Member

Cherrypicks, addresses merge conflicts, and squashes the commits from @heathbm in #79334, then adds an additional commit to clean up a few things in the src.

Fixes #76020
cc: @gfoidl, @MihaZupan, @tarekgh

@bartonjs, I looked at PemEncoding.TryFind, which is what drove us to add the decodingLength outs to IsValid, but it seems that implementation also needs to be able to find a base64-encoded value within the region rather than validating the whole thing?

heathbm and others added 2 commits May 8, 2023 13:24
@heathbm @stephentoub
Allow Base64Decoder to ignore space chars, add IsValid methods and tests
77b2628
@stephentoub
Some cleanup of Base64.IsValid changes
2347388 
This includes making FromBase64Transform significantly faster via SearchValues.
@stephentoub stephentoub added breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet labels May 8, 2023
@ghost
Copy link

ghost commented May 8, 2023 •
edited by stephentoub
Loading

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@ghost
Copy link

ghost commented May 8, 2023

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost ghost assigned stephentoub May 8, 2023
@ghost
Copy link

ghost commented May 8, 2023

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

Issue Details

Cherrypicks, addresses merge conflicts, and squashes the commits from @heathbm in #79334, then adds an additional commit to clean up a few things in the src.

Fixes #76020
cc: @gfoidl, @MihaZupan, @tarekgh

@bartonjs, I looked at PemEncoding.TryFind, which is what drove us to add the decodingLength outs to IsValid, but it seems that implementation also needs to be able to find a base64-encoded value within the region rather than validating the whole thing?

Author: stephentoub
Assignees: -
Labels:

area-System.Memory, breaking-change, new-api-needs-documentation, needs-breaking-change-doc-created
Milestone: -

MihaZupan
MihaZupan approved these changes May 8, 2023
View reviewed changes
@build-analysis build-analysis bot mentioned this pull request May 9, 2023
Closed
@stephentoub stephentoub removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label May 9, 2023
@stephentoub
Copy link
Member Author

Breaking change doc issue:
dotnet/docs#35282

@stephentoub
Copy link
Member Author

Failures are #85949

@stephentoub stephentoub merged commit b2d730c into dotnet:main May 9, 2023
@stephentoub stephentoub deleted the base64isvalid branch May 9, 2023 12:32
@stephentoub
Copy link
Member Author

I looked at PemEncoding.TryFind, which is what drove us to add the decodingLength outs to IsValid, but it seems that implementation also needs to be able to find a base64-encoded value within the region rather than validating the whole thing?

On closer look, seems it's just trimming whitespace in a somewhat convoluted manner. I'll submit a PR to replace it with this.

@EgorBo EgorBo mentioned this pull request May 16, 2023
Closed
@radekdoulik radekdoulik mentioned this pull request May 17, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jun 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@MihaZupan MihaZupan MihaZupan approved these changes

Assignees

@stephentoub stephentoub

Labels

area-System.Memory breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. new-api-needs-documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[API Proposal]: Base64.IsValid

3 participants

@stephentoub @MihaZupan @heathbm