-
-
Notifications
You must be signed in to change notification settings - Fork 226
Add more comprehensive tessts for A-labels in the "hostname" format #786
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not an expert on hostnames in general, but at least what I see seems to make sense to me
You've brought in restrictions from RFC5891 (e.g. "contains "--" in the 3rd and 4th position" -> invalid) which I don't think are correct. The hostname format is defined as a union of the requirements from RFC1123 and RFC5891, not an intersection, and "--" is a valid substring under RFC1123 as far as I can tell?
@karenetheridge The spec says,
Note that all strings valid against the "hostname" attribute are also valid against the "idn-hostname" attribute. Section 7.3.3
That means that any label in a hostname that starts with xn-- must be a valid IDN A-label. Otherwise it wouldn't be a valid idn-hostname.
The hostname format is defined as a union of the requirements from RFC1123 and RFC5891, not an intersection
I don't think that interpretation makes sense. A-labels (labels produced using the Punycode algorithm) are always valid RFC 1123 labels. So, that requirement would be meaningless if there were no requirement to validate those A-labels. That and the language that all hostnames are idn-hostnames is pretty convincing to me that hostnames need to be validated as ASCII-only idn-hostnames. I don't think it was a good idea to define it that way, but I think that's what the spec currently requires.
e17a314 to
b8bb858
Compare
@karenetheridge Is my explanation acceptable? Are you ok with with this moving forward?
Since draft-07, the
hostnameformat has specified that it includes IDNA2008 A-labels and that all validhostnames should also be valididn-hostnames. That means that it's not enough forhostnames to be valid punycode, they also need to be able to decode to a valid U-label including all the complex rules involved. This PR adds an equivalent A-label test for all of the U-label tests from theidn-hostnameformat tests to thehostnameformat tests.I also did a little clean up. I found several tests issues including a few duplicate tests, incorrect test descriptions, and few other issues. I also reordered some of the tests so they were grouped logically. I did the clean in a separate commit to make review a little easier.