http: unify header treatment#46528
Conversation
|
Review requested:
|
|
@climba03003 Are you referring to the test, right? |
I means the implementation. The trade-off is process the header with consistence encoding before combine to Lines 438 to 587 in a2a954b |
|
I knew about the optimization. I was double checking. |
Inside the code,
// from utf-8 to latin1
const first = Buffer.from('å', 'utf8').toString('latin1')
console.log(first, Buffer.from(first)) // <Buffer c3 83 c2 a5>
// here is what should be done before hand
const second = Buffer.from('å', 'latin1').toString('utf8')
console.log(second, Buffer.from(second)) // <Buffer ef bf bd>
// when data is latin1
const third = Buffer.from('ef bf bd', 'hex')
console.log(third, Buffer.from(third, 'latin1')) // <Buffer ef>
// when data is utf-8 or no encoding
const fouth = Buffer.from('ef bf bd', 'hex')
console.log(fouth, Buffer.from(fouth, 'utf8')) // <Buffer ef>Edit: hmm, buffer truncated doesn't seems correct. |
|
The problem is actually cause by Maybe special handling of |
|
@climba03003 I've changed it to check inside |
|
The RFC itself is a special case so I guess we have to act in "workaround way". LGTM. |
|
@nodejs/http Are we good on this? |
|
I think it's more of a bug fix than a breaking change... |
Per HTTP spec, header values are byte strings that should be decoded using isomorphic decode (latin1), not UTF-8. The new interceptor API (onResponseStart) was incorrectly using UTF-8. This change: - Updates parseHeaders and parseRawHeaders to use latin1 encoding - Removes the content-disposition workaround that was needed when headers were inconsistently decoded (see nodejs/node#46528) - Adds tests to verify latin1 decoding behavior Fixes #4753
resolves: #46395
I'm not sure what the testtest/parallel/test-http-server-response-standalone.jsimpacted by this change was trying to achieve @Trott