-
Notifications
You must be signed in to change notification settings - Fork 650
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
Clarify mediaType and artifactType syntax #1182
base: main
Are you sure you want to change the base?
Conversation
Media types values MUST have a top-level type and subtype name, separated by a `/`, without any parameters. | ||
|
||
The following regular expression may be used to validate media types: | ||
|
||
```regexp | ||
^[A-Za-z0-9][A-Za-z0-9!#$&^_.+-]{0,126}/[A-Za-z0-9][A-Za-z0-9!#$&^_.+-]{0,126}$ | ||
``` |
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.
Moving this to a common section seems good (instead of repeating it over and over again), but I'm not convinced we have a good reason to limit further than what the RFC specifies? To put that another way, what do we gain from adding extra restrictions on top of the RFC itself?
We've previously discussed (and don't explicitly say anything about) case sensitivity too, as another example: https://oci.dag.dev/?image=tianon/test:sPoNgEbOb
I don't think this is great, but the spec has historically pointed to the RFC as-is. I think it's totally fair for us to say our media types/spec-specified objects shouldn't have parameters, but something about telling other artifact authors they also cannot do so feels off.
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.
We were pointing directly to section 4.2, so there's a case to be made by some implementations that we had already excluded parameters that were defined in 4.3 (ECR rejects these manifests today). And schema/defs-descriptor.json
almost defined this as a restriction if there wasn't a bug in the regexp.
A big fear for allowing this for fields like artifactType includes the referrers query with an artifactType filter. Should that allow parameters in the query, or should all responses with any parameter be returned if no parameter is included? That part of the spec gets a bit undefined without this limit.
I was talking to my coworker @LaurentGoderre about this today, and he pointed me at https://developer.mozilla.org/en-US/docs/Web/Media/Formats/codecs_parameter and https://github.com/jsdom/whatwg-mimetype which have some of that "why is this a thing?" flavor I was suggesting we need to look for in one of the weekly OCI calls a while back. I don't think it's a perfect slam dunk "yes, we need to support these" but also sheds even more doubt (IMO) on whether we can reasonably apply a blanket "thou shalt not" on them either, so I'm personally leaning towards a NACK here. |
In the scenario of an artifact type set to Thinking of the risks of changing our mind in the future, allowing it today and denying it in the future would mean existing content on registries is no longer valid. But the reverse, denying it today and later deciding to allow it, would mean older registries and clients would reject new content. Neither is great, but the latter is at least feasible to me (I'd reject a change that invalidates existing valid data without a good reason). |
Signed-off-by: Brandon Mitchell <[email protected]>
92b19db
to
a2c8c4f
Compare
There was some confusion on whether parameters could be included in the mediaType or artifactType fields. This makes the following changes:
&-^
, which inadvertently included a long list of special characters that are not valid in a media type.