-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
Module loader does not allow parameters in data: URIs #30488
Comments
cc @bmeck iirc adding a compliant parser was blocked for some reason |
Correct, see #21128 |
A more progressive fix here might just be to extend the RegEx to support the specific cases in use here. I understand this is a slippery slope, but sometimes the small steps evolution approach can work best as opposed to blocking this on the larger effort (and both don't approaches aren't mutually exclusive so far as shipping goes - if the better approach is unblocked then that can win out). A small regex adjustment, something like - const DATA_URL_PATTERN = /^([^/]+\/[^,;]+)?(;charset=utf-?8)?(;base64)?,([\s\S]*)$/; may handle these common cases better. An explicit charset list handling seems sensible given that we don't currently support other charsets. @jbemmel if you'd be interested in working on a small PR to add this feature that would be very welcome. |
I was about to argue the same point, but my proposal would be something like this: const DATA_URL_PATTERN = /^([^/]+\/[^,]+?)?(;base64)?,([\s\S]*)$/; I quoted those examples because they are mentioned in the RFC, and I think NodeJS should at least allow those. However, my use case is slightly different - I would like to use data: URIs to build a http(s) module loader. In that context, I need to retain the URL from which a given data: URI was fetched - so I added a custom 'url' parameter, which gets rejected. |
A PR has been opened to ignore parameters based upon the WHATWG splitting point for |
This should be fixed in master, please be aware you will need to use URL encoding for anything with a |
Hey, sorry it took so long for this to be closed, but I think this is resolved. If this (or a similar error) is still occuring. Please open a new issue, as this one is outdated. |
The regex at https://github.com/nodejs/node/blob/master/lib/internal/modules/esm/translators.js#L42 defines the allowable data: URIs:
This rejects valid RFC2397 data: URIs with media parameters, like
data:text/javascript;charset=utf-8;base64,xxxx
Likewise, a valid URI like
data:,A%20brief%20note
is rejected ( '/' character is required in the media type - even though the media type is not used )
The text was updated successfully, but these errors were encountered: