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

spec: define charset for "org.opencontainers.ref.name" #599

Closed
stevvooe opened this issue Mar 8, 2017 · 18 comments · Fixed by #671
Closed

spec: define charset for "org.opencontainers.ref.name" #599

stevvooe opened this issue Mar 8, 2017 · 18 comments · Fixed by #671
Milestone

Comments

@stevvooe
Copy link
Contributor

stevvooe commented Mar 8, 2017

Currently, there is no restriction on the character set for reference names. We'll need something open but define some common bounds to make this.

I suggested restricting this to the URI character set, in #591 (comment).

Problem called out in #591.

@stevvooe
Copy link
Contributor Author

stevvooe commented Mar 8, 2017

cc @q384566678

@stevvooe stevvooe added this to the v1.0.0-rc6 milestone Mar 8, 2017
@vbatts
Copy link
Member

vbatts commented Mar 9, 2017

cc @alexlarsson, would the URI character set work for you as well?

@jonboulle
Copy link
Contributor

To be very explicit, is this a change in direction from the position expressed here and here?

@stevvooe
Copy link
Contributor Author

stevvooe commented Mar 9, 2017

@jonboulle Not exactly and there is some subtlety here.

From the perspective of the specification, annotations are opaque. They will be read and propagated without modification or verification.

This does not mean that the specifier of the annotation can't make requirements that a given annotation has a structure that is internal to that annotation. Practically, this means that when an annotation producer or consumer reads an annotation, it may be generated or validated in a specified format.

Let me know if this distinction is unclear. It seems like splitting hairs, but it is to avoid having the annotations "leak" into the first-class field space.

@zhouhao3
Copy link

I think that as long as the string type of standard can be, do not need more norms.

@erikh
Copy link
Contributor

erikh commented Mar 23, 2017

https://en.m.wikipedia.org/wiki/Punycode Is a method of encoding non-latin encodings into urls in a way that's easy to both parse and display in all instances. Maybe that's a good solution since it has a limited input set?

@erikh
Copy link
Contributor

erikh commented Mar 23, 2017

It's used in many internationalized urls, so it's not just a spec for spec's sake. :D

@erikh
Copy link
Contributor

erikh commented Mar 23, 2017

https://godoc.org/golang.org/x/net/idna Is a golang library to manage the conversion.

@AkihiroSuda
Copy link
Member

How to tell punycode string from ASCII string?
(Is is valid to use "xn--" prefix for non-domain strings?)

Why not use percent-encoded UTF-8?

@erikh
Copy link
Contributor

erikh commented Apr 26, 2017

no argument from me re: percent encoded utf-8

@erikh
Copy link
Contributor

erikh commented Apr 28, 2017

So I did some reading tonight and discovered these. With the punycode suggestion I was attempting to find a word-safe set of characters to use and realized later that the unicode standard already does this for us elsewhere. :)

http://unicode.org/reports/tr18/#Categories

Which are supported by most regex implementations these days, since unicode classes are well-defined.

See also: https://golang.org/pkg/unicode/

We could use subsets of these supported regex classes to define our character set; the reason I suggest this is that it'd be an insanely painful exercise to satisfy all needs here by defining it manually.

So, presuming this is a suitable solution, we really just need to cherry-pick classes from the listings. I would start the discussion with digits+alpha+marks, maybe. No strong opinions myself, and would love to hear from people actually storing/indexing by name.

@erikh
Copy link
Contributor

erikh commented Apr 28, 2017

while I'm here, I think it might be useful to have annotations (either as struct tags in code that are managed or just in the spec) that define this clearly for all named string fields in the spec, and also for general unnamed sections (like annotations in the spec).

e.g., the digests should have a charset, the chainIDs, the tags, etc.

@erikh
Copy link
Contributor

erikh commented Apr 28, 2017

one more thing: I realize that some of these fields are obvious, but it doesn't hurt too much to spell it out; one thing I've been thinking about with this project lately is the dependency on go libraries that are not 100% clear on their operation (which I'm not expecting to change, because it's unreasonable to expect it) and the best we can really do is make sure the spec is bulletproof when it comes to different language implementations.

@vbatts
Copy link
Member

vbatts commented May 3, 2017

sounds like this got a bit complicated. I'm still thinking that URI character set is suitable, and haven't heard clearly that it is not suitable. Let's move forward with that.

@erikh
Copy link
Contributor

erikh commented May 3, 2017 via email

@vbatts
Copy link
Member

vbatts commented May 3, 2017

@erikh don't be sorry!

vbatts added a commit to vbatts/oci-image-spec that referenced this issue May 11, 2017
URI are standard enough. Though the most concise character set for the
URI is in the appendix A https://tools.ietf.org/html/rfc3986#appendix-A
unless I missed something.

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
@vbatts
Copy link
Member

vbatts commented May 11, 2017

#671

vbatts added a commit to vbatts/oci-image-spec that referenced this issue May 12, 2017
URI are standard enough. Though the most concise character set for the
URI is in the appendix A https://tools.ietf.org/html/rfc3986#appendix-A
unless I missed something.

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
vbatts added a commit to vbatts/oci-image-spec that referenced this issue May 17, 2017
URI are standard enough. Though the most concise character set for the
URI is in the https://tools.ietf.org/html/rfc3986 unless I missed something.

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
vbatts added a commit to vbatts/oci-image-spec that referenced this issue May 18, 2017
URI are standard enough, though % opens it a bit too much.
After much discussion we're going to keep it as close to the docker tag
as possible but with +, @ and /
reference: opencontainers#671 (comment)

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
vbatts added a commit to vbatts/oci-image-spec that referenced this issue May 19, 2017
URI are standard enough, though % opens it a bit too much.
After much discussion we're going to keep it as close to the docker tag
as possible but with +, @ and /
reference: opencontainers#671 (comment)

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
@stevvooe stevvooe modified the milestones: v1.0.0, v1.0.0-rc6 May 19, 2017
vbatts added a commit to vbatts/oci-image-spec that referenced this issue May 25, 2017
URI are standard enough, though % opens it a bit too much.
After much discussion we're going to keep it as close to the docker tag
as possible but with +, @ and /
reference: opencontainers#671 (comment)

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
vbatts added a commit to vbatts/oci-image-spec that referenced this issue May 25, 2017
URI are standard enough, though % opens it a bit too much.
After much discussion we're going to keep it as close to the docker tag
as possible but with +, @ and /
reference: opencontainers#671 (comment)

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
@AkihiroSuda
Copy link
Member

As of rc6, the correct annotation key is "org.opencontainers.image.ref.name"!
https://github.com/opencontainers/image-spec/blob/v1.0.0-rc6/image-layout.md
Anyone please fix the issue title so that people won't copy-paste by mistake? 😅

vbatts added a commit to vbatts/oci-image-spec that referenced this issue Jun 1, 2017
URI are standard enough, though % opens it a bit too much.
After much discussion we're going to keep it as close to the docker tag
as possible but with +, @ and /
reference: opencontainers#671 (comment)

And rather than making up our own mock grammar, lean on more formal
EBNF.

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
vbatts added a commit to vbatts/oci-image-spec that referenced this issue Jun 2, 2017
URI are standard enough, though % opens it a bit too much.
After much discussion we're going to keep it as close to the docker tag
as possible but with +, @ and /
reference: opencontainers#671 (comment)

Fixes opencontainers#599

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants