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

MIME type overmatch in data URLs #48957

Closed
ghost opened this issue Jul 28, 2023 · 6 comments · Fixed by #49104
Closed

MIME type overmatch in data URLs #48957

ghost opened this issue Jul 28, 2023 · 6 comments · Fixed by #49104
Labels
confirmed-bug Issues with confirmed bugs. esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@ghost
Copy link

ghost commented Jul 28, 2023

Version

v20.5.0

Platform

Linux host 5.19.0-45-generic #46-Ubuntu SMP PREEMPT_DYNAMIC Wed Jun 7 09:08:58 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

modules

What steps will reproduce the bug?

Execute this command.

node --input-type=module --eval 'import "data:ONCEUPONAtext/javascriptTHEREWASASNEAKYMODULE,console.log(\"EVALUATED\")"'

The incorrectly typed inline module is evaluated. The string is logged to the console.

EVALUATED

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

Consistently.

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

An incorrect MIME type is forbidden.

What do you see instead?

The module is evaluated.

Additional information

formats.js has a regex matching the MIME type.

/\s*(text|application)\/javascript\s*(;\s*charset=utf-?8\s*)?/i

This will match any MIME type that contains (text|application)/javascript anywhere. It maybe needs start and end anchors.

@aduh95
Copy link
Contributor

aduh95 commented Jul 29, 2023

Good catch. FYI if you paste a "Permalink URL", it will show the line(s) of code:

/\s*(text|application)\/javascript\s*(;\s*charset=utf-?8\s*)?/i,

Would you like to send a PR to fix this?

@aduh95 aduh95 added confirmed-bug Issues with confirmed bugs. esm Issues and PRs related to the ECMAScript Modules implementation. labels Jul 29, 2023
@ghost
Copy link
Author

ghost commented Jul 29, 2023

No thanks, I won't be able to get to this.

@ntedgi
Copy link
Contributor

ntedgi commented Jul 29, 2023

@aduh95 i will be happy to help with this bug

@lzcampos
Copy link

lzcampos commented Aug 1, 2023

@aduh95 i will be happy to help with this bug

I would be glad helping with this, too. Feel free to reach me if you want to.

@Xstoudi
Copy link
Contributor

Xstoudi commented Aug 9, 2023

I started working on it, but I'm figuring out that the MIME type may be wrong overall.

The current regexp also allow application/javascript while the doc clearly indicate that only text/javascript is valid: https://nodejs.org/api/esm.html#data-imports

As https://datatracker.ietf.org/doc/html/rfc9239 stands that text/javascript is now common and application/javascript is obsolete and the PR was made when it wasn't the case, I'll submit a PR changing that to get comment about this specific point.

@andremralves
Copy link
Contributor

andremralves commented Aug 11, 2023

As the previous PR got closed and it seems that nobody is currently working on this issue I would like to submit a PR :)

Edit: The change is pretty simple, I'm now creating a test case.

andremralves added a commit to andremralves/node that referenced this issue Aug 11, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
data MIME types.
Fixes: nodejs#48957
andremralves added a commit to andremralves/node that referenced this issue Aug 11, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for "data:URLs".
Fixes: nodejs#48957
andremralves added a commit to andremralves/node that referenced this issue Aug 11, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for "data:URLs".
Fixes: nodejs#48957
aduh95 pushed a commit to andremralves/node that referenced this issue Aug 13, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for "data:URLs".
Fixes: nodejs#48957
aduh95 pushed a commit to andremralves/node that referenced this issue Aug 13, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for "data:URLs".
Fixes: nodejs#48957
aduh95 pushed a commit that referenced this issue Aug 13, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: #49104
Fixes: #48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: nodejs#49104
Fixes: nodejs#48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit that referenced this issue Aug 15, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: #49104
Fixes: #48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit to RafaelGSS/node that referenced this issue Aug 15, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: nodejs#49104
Fixes: nodejs#48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
rluvaton pushed a commit to rluvaton/node that referenced this issue Aug 15, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: nodejs#49104
Fixes: nodejs#48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit that referenced this issue Aug 16, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: #49104
Fixes: #48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit that referenced this issue Aug 17, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: #49104
Fixes: #48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this issue Nov 27, 2023
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: #49104
Fixes: #48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: nodejs/node#49104
Fixes: nodejs/node#48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: nodejs/node#49104
Fixes: nodejs/node#48957
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
5 participants