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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make File's type more strict #1703

Merged
merged 1 commit into from Oct 15, 2022
Merged

Conversation

KhafraDev
Copy link
Member

@KhafraDev KhafraDev commented Oct 14, 2022

This might be a bad change because, although the spec says to make type a "parseable MIME type", browsers have seemingly ignored this for years (see: nodejs/node#45008 (comment)).

Not only this, it

  1. causes more tests to fail (as Blob's attribute is still 'compliant' with browsers I guess?)
  2. requires us to implement our own type getter, rather than using Blob's.
  3. causes a FileAPI test to fail. I don't think it was meant to pass, as it is literally named "nonparseable" though.

With this change, if you remove the Blob tests from the parsing.any.js test, ~400 more tests succeed. This is because the File and Blob tests are grouped together and failing an assert causes the File test not to run. 馃槥

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mcollina mcollina merged commit 8d6ddb7 into nodejs:main Oct 15, 2022
@KhafraDev KhafraDev deleted the fix-file-type branch October 15, 2022 14:29
metcoder95 pushed a commit to metcoder95/undici that referenced this pull request Dec 26, 2022
crysmags pushed a commit to crysmags/undici that referenced this pull request Feb 27, 2024
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

Successfully merging this pull request may close these issues.

None yet

3 participants