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

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

Merged
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
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

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.

@dotnet-issue-labeler
Copy link

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: -

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

Successfully merging this pull request may close these issues.

[API Proposal]: Base64.IsValid
3 participants