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

fix(stdlib): ensure punycode functions keep all input parts #710

Merged

Conversation

esensar
Copy link
Contributor

@esensar esensar commented Feb 22, 2024

I have implemented validation skip in #709, which ignores errors returned by idna crate when encoding and decoding punycode. One of the errors that it could return would result in output with missing sections (as was demonstrated in tests - e.g. multiple_errors_ignore). Since it is fairly trivial to split domain into parts (idna crate does it by doing split on .), we can split the domain ourselves in cases when we don't want validation and then try to encode ones that need encoding and leave them as they were if encoding (or decoding) fails.

Related: #709

…tion is off

I have implemented validation skip in vectordotdev#709, which ignores errors returned by `idna` crate when
encoding and decoding punycode. One of the errors that it could return would result in output with
missing sections (as was demonstrated in tests - e.g. `multiple_errors_ignore`). Since it is fairly
trivial to split domain into parts (`idna` crate does it by doing split on `.`), we can split the
domain ourselves in cases when we don't want validation and then try to encode ones that need
encoding and leave them as they were if encoding (or decoding) fails.

Related: vectordotdev#709
@jszwedko jszwedko requested a review from pront February 22, 2024 14:35
@esensar
Copy link
Contributor Author

esensar commented Feb 26, 2024

I haven't added a changelog since it affects changes that were already covered by a changelog entry that wasn't yet released. Is that alright or should I add one?

@jszwedko
Copy link
Member

I haven't added a changelog since it affects changes that were already covered by a changelog entry that wasn't yet released. Is that alright or should I add one?

👍 that seems right since the other change hasn't been released yet.

@jszwedko jszwedko added the no-changelog Changes in this PR do not need user-facing explanations in the release changelog label Feb 26, 2024
@jszwedko jszwedko added this pull request to the merge queue Feb 27, 2024
Merged via the queue into vectordotdev:main with commit 3941fb6 Feb 27, 2024
13 of 15 checks passed
@esensar esensar deleted the enhancement/punycode_ignore_validation_fix branch February 27, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Changes in this PR do not need user-facing explanations in the release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants