[idna] Preserve leading dots in host #337
Closed
Conversation
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
|
r? @SimonSapin |
|
@behnam Please be patient. :) |
A retry of servo#171 This diff changes the behavior of ToASCII step to match the spec and prevent failures on some cases when a domain name starts with leading dots (FULL STOPs), as requested in servo#166. The change in the code results in a few failures for test cases of the Conformance Testing data provided with UTS servo#46. But, as the header of the test data file (IdnaTest.txt) says: "If the file does not indicate an error, then the implementation must either have an error, or must have a matching result." Therefore, failing on those test cases does not break conformance with UTS servo#46, and to some level, anticipated. As mentioned in servo#166, a feedback is submitted for this inconsistency and the test logic can be improved later if the data file addresses the comments. Until then, we can throw less errors and maintain passing conformance tests with this diff. To keep the side-effects of ignoring errors during test runs as minimum as possible, I have separated `TooShortForDns` error from `TooLongForDns`. The `Error` struct has been kept private, so the change won't affect any library users. Fix servo#166
This was referenced May 24, 2017
|
New implementation of this, plus lots of other updates to improve validation (bidi) rules is here: #351 Closing this, as looks like there's no interest for a quick and dirty fix. |
bors-servo
pushed a commit
that referenced
this pull request
Jun 21, 2017
[idna] Update data to Unicode 10.0 and fix logic * Change the behavior of ToASCII step to match the spec and prevent failures on some cases when a domain name starts with leading dots (FULL STOPs), as requested in #166. (Another attempt on #337 and #171) * Update `IdnaTest.txt` file to UCD 10.0 and fix Validation Rules, specially Bidi Rules, for the tests to pass. * Add TODO marks for new flags introduced in Unicode 10.0 version of UTS#46. (http://www.unicode.org/reports/tr46/proposed.html) * Add integration test for `rust-url` crate for the new behavior. Fix #166 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/351) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A retry of #171
This diff changes the behavior of ToASCII step to match the spec and
prevent failures on some cases when a domain name starts with leading
dots (FULL STOPs), as requested in
#166.
The change in the code results in a few failures for test cases of the
Conformance Testing data provided with UTS #46. But, as the header of
the test data file (IdnaTest.txt) says: "If the file does not indicate
an error, then the implementation must either have an error, or must
have a matching result."
Therefore, failing on those test cases does not break conformance with
UTS #46, and to some level, anticipated.
As mentioned in #166, a feedback
is submitted for this inconsistency and the test logic can be improved
later if the data file addresses the comments.
Until then, we can throw less errors and maintain passing conformance
tests with this diff.
To keep the side-effects of ignoring errors during test runs as minimum
as possible, I have separated
TooShortForDnserror fromTooLongForDns. TheErrorstruct has been kept private, so the changewon't affect any library users.
Fix #166
This change is