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

XMLHttpRequest/data-uri.htm uses invalid URLs #7374

Open
asankah opened this issue Sep 18, 2017 · 10 comments
Open

XMLHttpRequest/data-uri.htm uses invalid URLs #7374

asankah opened this issue Sep 18, 2017 · 10 comments
Labels

Comments

@asankah
Copy link
Contributor

asankah commented Sep 18, 2017

The tests in data-uri.htm use URLs with unescaped spaces. Consider using percent escaping so that the handling of invalid URLs don't affect the test outcome.

@domenic
Copy link
Member

domenic commented Sep 26, 2017

I think the point is to test "invalid" URL handling, among others. /cc @annevk

@asankah
Copy link
Contributor Author

asankah commented Sep 26, 2017

The invalid URL is the only URL though. Which is why I assumed that it was not intentional.

@annevk annevk added the xhr label Sep 27, 2017
@annevk
Copy link
Member

annevk commented Sep 27, 2017

Most URLs there have unescaped spaces and as that has resulted in browser difference being uncovered it's very much intentional. (This is generally true for all tests. Tests need to cover unexpected inputs as websites will give browsers unexpected inputs. That's also why standards define how to handle such inputs.)

@annevk annevk closed this as completed Sep 27, 2017
@asankah
Copy link
Contributor Author

asankah commented Sep 27, 2017

Are you saying that the XHR tests are where you are also testing spec compliancy of data URI parsing?

Please consider being consistent about what is being tested where. It's fine to test spec behavior WRT invalid inputs where such behavior is specified (where is invalid URL for data URIs specified?), but having that as the only test case is a bit absurd.

@annevk
Copy link
Member

annevk commented Sep 28, 2017

@asankah you cannot test this all in isolation. How would you observe a data URL response body without something like XMLHttpRequest?

@RByers
Copy link
Contributor

RByers commented Oct 27, 2017

@annevk, I see this test is failing with spaces being stripped on all browsers except Edge. Thank you for working to specify the behavior here, but prior to that landing is this unescaped space behavior actually specified anywhere?

If not, then perhaps it's better to change this test to not rely on spaces for now, and keep the debate about how exactly they should be handled to your new spec and tests?

@RByers RByers reopened this Oct 27, 2017
@annevk
Copy link
Member

annevk commented Nov 2, 2017

@RByers I don't think it's a good idea to remove tests that demonstrate interoperability issues.

Given that browsers accept spaces in URLs and https://tools.ietf.org/html/rfc2397 doesn't say that MIME type specific processing is appropriate, it seems most logical to me that if you don't strip spaces for text/plain you also shouldn't strip them for image/png.

I'd much rather have a discussion about the underlying issue and if there's disagreement with my proposed solution for it, than about issues that arise because the underlying issue isn't solved.

Furthermore, the main reason whatwg/fetch#579 (the new data URL standard) hasn't landed yet is because of MIME type parsing, which I'm still researching and filing numerous browser bugs on. Nobody thus far disagreed with how spaces ought to be handled. If you think we need different handling for spaces that would be the place to argue for that.

@annevk
Copy link
Member

annevk commented Nov 2, 2017

FWIW, I guess I won't object if you want to mark this test tentative for a while, but it feels a lot like busywork especially given that it's been quite hard thus far to get feedback on how to fix the relevant standards themselves. It seems to me we should all be primarily concerned with that.

@asankah
Copy link
Contributor Author

asankah commented Nov 2, 2017

@annevk : It would go a long way if you explain why this demonstrates an interop issue.

@annevk
Copy link
Member

annevk commented Nov 2, 2017

@asankah if there's an observable difference between implementations there's an interop issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants