nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
105.24k stars 28.51k forks source link

Blob's type should not be all lowercased #42266

Open jimmywarting opened 2 years ago

jimmywarting commented 2 years ago

(only parts of it should be lowercased)

Version

17.5

Platform

mac

Subsystem

No response

What steps will reproduce the bug?

A meta value can contain uppercase letters that are important to be kept as is there is an ongoing issue about this in the spec too about this.

const type = 'multipart/form-data; boundary=----WebKitFormBoundaryDUbbET022Y1HGX5x'
new Blob(payload, { type }).type === type // false

The only thing that should be lowercased is the mimetype (multipart/form-data) and the keys (boundary) the value is important to keep it as is. otherwise you would not be able to parse the payload correctly as you lose valuable information.

fetch-blob is more relaxed and don't cast anything to lowercase (as we are a bit lazy and don't have a proper mime type parser like whatwg-mimetype) so we only keep the type string as is...

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

every time

What is the expected behavior?

to not lowercase the boundary value and instead keep it as is (and yield true instead.

const type = 'multipart/form-data; boundary=----WebKitFormBoundaryDUbbET022Y1HGX5x'
new Blob(payload, { type }).type === type // true
const type = 'mUltiPart/foRm-dAta; boundary=----WebKitFormBoundaryDUbbET022Y1HGX5x'
new Blob(payload, { type }).type === 'multipart/form-data; boundary=----WebKitFormBoundaryDUbbET022Y1HGX5x'

What do you see instead?

every char in type is lowercased

Additional information

https://github.com/w3c/FileAPI/issues/43

rayw000 commented 2 years ago

rfc2046#section-5.1 didn't say anything clearly about whether the parameter boundary should be case-sensitive or not, but examples in this section look like case-sensitive.

Currently Chrome Version 99.0, Firefox 98.0 and Safari 15.1 are all doing this case conversion on boundary when creating a new Blob using your sample code snippet. I'm afraid fixing this issue may bring a breaking change.

Perhaps we could keep only the original case of boundary only when we see both multipart and boundary. I'm not sure how heavy it will impact the existing JavaScript code.

Since you are the author here, how do you think of it? @targos https://github.com/nodejs/node/blob/625c1e841adee3e687225597e60c70df6510a696/lib/internal/blob.js#L166-L167

jimmywarting commented 2 years ago

it's not only about the boundary value, it can be anything x/y; x-foo=BAR

rayw000 commented 2 years ago

it's not only about the boundary value, it can be anything x/y; x-foo=BAR

Yes, but there are only finite meaningful parameters defined in rfc2046, and some of them have already been clearly defined as case-insensitive, for example charset and access-type.

rayw000 commented 2 years ago

I guess the essence of this issue is, many browsers generate case-sensitive boundary when submitting multipart requests, while Node.js converts them into case-insensitive ones, which breaks the === comparison.

So, for the parameter boundary, we could keep its original case. For other parameters like x-foo=BAR you mentioned, I think the application which brings this parameter should be responsible for this.