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

Consider using the basic URL parser (i.e. disallowing blob URLs) #38

Closed
domenic opened this issue Jun 20, 2017 · 2 comments
Closed

Consider using the basic URL parser (i.e. disallowing blob URLs) #38

domenic opened this issue Jun 20, 2017 · 2 comments

Comments

@domenic
Copy link

domenic commented Jun 20, 2017

Since these are being passed to other systems, blob URLs might not make sense there.

On the other hand, maybe they would, e.g. when web share targets are available. So maybe this is fine. But I wanted to raise the possibility.

@mgiuca
Copy link
Collaborator

mgiuca commented Jun 20, 2017

I don't think it matters either way. Because I am not parsing a URL string to a URL record, then transmitting the URL record (which would have a blob in the URL parser case). I am serializing the URL record back to a URL string again. So using URL Parser vs Basic URL Parser should be equivalent (blob URLs are implicitly allowed, but we would not transmit the blob object).

URL Parser seemed a bit higher level which is why I went with it. Should I still change to Basic URL Parser?

If you want to disallow blob URLs, I think we need an explicit check for it and throw a TypeError.

@domenic
Copy link
Author

domenic commented Jun 20, 2017

You're right on both counts; the parser used doesn't matter in this case, and my suggestion doesn't properly disallow blobs.

Probably leaving things scheme-agnostic makes more sense then, if there's no natural machinery for excluding blobs. So yeah, I'll close; thanks for setting me straight!

@domenic domenic closed this as completed Jun 20, 2017
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