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

Valid URI-reference #195

Closed
annevk opened this issue Jan 13, 2020 · 9 comments
Closed

Valid URI-reference #195

annevk opened this issue Jan 13, 2020 · 9 comments

Comments

@annevk
Copy link
Member

annevk commented Jan 13, 2020

I don't think any browser has a primitive for this definition so I think we should rely on it. Instead we should use the URL parser with a base URL and throw away failures (and perhaps report those to the developer console).

From: https://w3c.github.io/reporting/structured-headers/#header.

@clelland
Copy link
Contributor

That text was shamelessly stolen from https://tools.ietf.org/id/draft-ietf-httpbis-header-structure-14.html#rfc.section.2; I suppose it's a mismatch between HTTP-level headers and HTML-level specs using them.

I'll update to refer to URL parser instead.

@annevk
Copy link
Member Author

annevk commented Jan 13, 2020

Yeah, see also the Location header and such (though we still haven't quite defined how that works).

@clelland
Copy link
Contributor

clelland commented Jan 13, 2020

The best I can find for a definition is "Valid URL string" from URL; the processing algorithm does already specify that we use the URL parser: https://w3c.github.io/reporting/structured-headers/index.html#ref-for-dom-endpoint-url, although it doesn't say what to do on failure.

Edit: We actually use "absolute-url string or path-absolute-url string" later in the text, which is more restrictive than "valid URL string", so I switched to that for consistency.

@annevk
Copy link
Member Author

annevk commented Jan 13, 2020

If there's a processing algorithm you cannot also say here that something MUST be ignored as that would contradict the processing algorithm, no? You don't wanna require something twice (and in this case it would likely not be the same requirement, even).

@clelland
Copy link
Contributor

The processing algorithm causes it to be ignored, with:

If endpoint url is not a string, or if that value is not an absolute-URL string or a path-absolute-URL string, skip to the next endpoint item.

But you're right -- "will be ignored" would be better, as would not saying anything at all here. The algorithm should be normative, and the rest of the text shouldn't be in the business of spelling out consequences.

@annevk
Copy link
Member Author

annevk commented Jan 14, 2020

So that's also a bug in the processing algorithm as those productions are really only meant for producers. A consumer should always go through the URL parser.

@clelland
Copy link
Contributor

This text originally came from #147, and exists today in Reporting.
Is there a better way to restrict the values that pass in to the parser? We don't want, for instance, data: or blob:, or file: URLs to be specified as endpoints. Relative URLs starting with a "/" should be allowed, per #147, but we didn't want to support other relative URLs at the time that was decided.

Since this is just for per-document reports, relative vs. path-absolute might not be an issue anymore, but I don't think we want to open it up to any string that happens to get through the parser.

@annevk
Copy link
Member Author

annevk commented Jan 14, 2020

You can safelist certain schemes, but you want to do so after parsing. Safelisting before parsing is rather unsafe.

@clelland
Copy link
Contributor

clelland commented Feb 10, 2020

Okay, changed, removed pre-filtering in favor of just checking for potential trustworthiness after parsing.

Latest update in #197

@clelland clelland closed this as completed Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants