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

Data URLs can't have query params #54944

Closed
avivkeller opened this issue Sep 14, 2024 · 6 comments
Closed

Data URLs can't have query params #54944

avivkeller opened this issue Sep 14, 2024 · 6 comments
Labels
fetch Issues and PRs related to the Fetch API loaders Issues and PRs related to ES module loaders

Comments

@avivkeller
Copy link
Member

Version

main

Platform

N/A

Subsystem

No response

What steps will reproduce the bug?

await import("data:text/javascript,console.log(import.meta.url)?query")

How often does it reproduce? Is there a required condition?

Everytime

What is the expected behavior? Why is that the expected behavior?

The Data URL should load correctly, printing the import.meta.url with the query

What do you see instead?

SyntaxError: Unexpected end of input

Additional information

I believe this is due to #54748 (CC @KhafraDev)

@avivkeller avivkeller added confirmed-bug Issues with confirmed bugs. loaders Issues and PRs related to ES module loaders fetch Issues and PRs related to the Fetch API labels Sep 14, 2024
@KhafraDev KhafraDev removed the confirmed-bug Issues with confirmed bugs. label Sep 14, 2024
@KhafraDev
Copy link
Member

KhafraDev commented Sep 14, 2024

The parser is correct here. It doesn't work in Chrome or Firefox nor could I find anywhere in the spec that says to treat query params separately from the body.

If I'm wrong please correct me.

Treating query params separately creates a myriad of issues that moving to a dedicated parser aimed to fix; should the question mark in data:text/javascript;console.log('?') be treated as a query param?

@avivkeller
Copy link
Member Author

Shouldn't that PR become semver-major, as it modifies the existing behavior?

@KhafraDev
Copy link
Member

I'd consider it a bug fix so not a semver-major change, we've done this a few times in undici. @nodejs/web-standards

@aduh95
Copy link
Contributor

aduh95 commented Sep 15, 2024

Someone reported the old behavior as a bug: #53775

It's always tricky to decide whether any change qualifies as a bug fix or as a breaking change (cf XKCD 1172). Do you have a use case for having a query parameter?

@avivkeller
Copy link
Member Author

Do you have a use case for having a query parameter?

Not really, I just noticed that this behavior was different in the main branch than the latest release when preparing #54933, and I figured it should be a non-patch change when sent to release lines.

@aduh95
Copy link
Contributor

aduh95 commented Sep 15, 2024

IMO we should treat it as a bug fix (i.e. semver-patch): if there’s no use case for it, then it’s not going to break anyone — and it’s going to unbreak a few folks

@avivkeller avivkeller closed this as not planned Won't fix, can't repro, duplicate, stale Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fetch Issues and PRs related to the Fetch API loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

No branches or pull requests

3 participants