Fix a grammar mistake in serialization.md and don't imply _ is valid in dns hostnames #163
Conversation
|
Can we reference an RFC(s)? "must follow standard rules" is too vague imho.. |
|
On Tue, Jun 21, 2016 at 02:41:43AM -0700, Jonathan Boulle wrote:
I'm not sure how specific the hostname restriction is intended to be, |
|
+1 on referencing the RFC @jonboulle |
|
Ping @jbouzane |
|
@jbouzane Final ping before I take this one over. |
|
Take it over On Wed, Aug 10, 2016 at 12:55 PM, Brandon Philips [email protected]
|
|
Please take another look. |
|
@jbouzane lives! 🙌 |
|
updates look good, but git commits need a Signed-off-by: |
| @@ -75,7 +75,7 @@ This specification uses the following terms: | |||
| A collection of tags grouped under a common prefix (the name component before <code>:</code>). | |||
| For example, in an image tagged with the name <code>my-app:3.1.4</code>, <code>my-app</code> is the <i>Repository</i> component of the name. | |||
| A repository name is made up of slash-separated name components, optionally prefixed by a DNS hostname. | |||
| The hostname must follow comply with standard DNS rules, but may not contain <code>_</code> characters. | |||
| The hostname must comply with standard DNS rules that apply to LDH or IDNA labels as defined in <a href="https://tools.ietf.org/html/rfc5890#section-2.3">RFC 5890</a>. | |||
There was a problem hiding this comment.
From RFC 5890, LDH labels include “Fake A-labels”, which I don't think we want to allow (at least, not if we are putting DNS-related constraints on hostnames, which is what this PR is about). Later in the RFC:
For IDNA-aware applications, the three types of valid labels are "A-labels", "U-labels", and "NR-LDH labels", each of which is defined below.
So I think we want to echo that and say:
The hostname must be an A-label, U-label, or NR-LDH label, as defined in RFC 5890.
And we are not supporting binary labels, which RFC 5890 lists as a non-ASCII label not covered under IDNA? I'm personally agnostic on refusing or allowing binary labels, but want to make sure that we're making an explicit decision one way or the other.
Signed-off-by: Jason Bouzane <[email protected]>
|
I'll buy that. |
|
LGTM |
1 similar comment
|
LGTM |
| @@ -75,7 +75,7 @@ This specification uses the following terms: | |||
| A collection of tags grouped under a common prefix (the name component before <code>:</code>). | |||
| For example, in an image tagged with the name <code>my-app:3.1.4</code>, <code>my-app</code> is the <i>Repository</i> component of the name. | |||
| A repository name is made up of slash-separated name components, optionally prefixed by a DNS hostname. | |||
| The hostname must follow comply with standard DNS rules, but may not contain <code>_</code> characters. | |||
| The hostname must comply with standard DNS rules that apply to IDNA labels as defined in <a href="https://tools.ietf.org/html/rfc5890#section-2.3">RFC 5890</a>. | |||
There was a problem hiding this comment.
I think we want to explicitly list A-label, U-label, or NR-LDH label. Searching the spec for a definition for “IDNA labels” turned up this:
IDNA labels come in two flavors: an ACE-encoded form and a Unicode (native character) form. These are referred to as A-labels and U-labels, respectively…
But I think we want to allow NR-LDH labels as well, because U-labels must contain at least one non-ASCII character, and I'm fine with all-ASCII, non-A label, repository names.
|
i'm not sure why travis is testing the wrong commit. and why pull-approve seems like it's out for lunch :-\ |
|
On Wed, Aug 24, 2016 at 10:55:21AM -0700, Vincent Batts wrote:
The unsigned a52d250 (#188) has landed in master since 9e7f05f's |
|
@caniszczyk I have no idea what is wrong but we can't merge this. Can you force it or something? |
|
@caniszczyk this is the tool messing up. Can you please just force merge this? |
|
I am going to try and work around this with #226 |
|
Closing in favor of #226 which doesn't have broken travis stuff. |
Signed-off-by: Jason Bouzane [email protected]