Handle OPTIONS content-length #83
Conversation
| if (headers['content-length']) { | ||
| headers = filterHeaders(headers, 'content-length') | ||
| if (req.method === 'GET' || req.method === 'HEAD' || req.method === 'OPTIONS') { | ||
| if (headers.hasOwnProperty('content-length')) { |
There was a problem hiding this comment.
No. If you want to use hasOwnProperty, than do it with
Object.prototype.hasOwnProperty.call(headers, 'content-length')
But imho the better way is
'content-length' in headers
There was a problem hiding this comment.
Right, safer and better to use in
| headers = filterHeaders(headers, 'content-length') | ||
| if (req.method === 'GET' || req.method === 'HEAD' || req.method === 'OPTIONS') { | ||
| if (headers.hasOwnProperty('content-length')) { | ||
| delete headers['content-length'] |
There was a problem hiding this comment.
Using filterHeaders has maybe some deeper reason.
There was a problem hiding this comment.
Here's the context when it was introduced #38 (for the same undici reason I raised about OPTIONS)
-We could think it was made that way to prevent object mutation but headers are taken from a spread, so no risks of modifying elsewhere, and headers object is already mutated
headers['x-forwarded-host'] = headers.host
headers.host = url.hostname
-filterHeaders ensures a case insensitive removal, but first, headers are supposed to already be lowercase and anyway, a case-sensitive comparison is made before calling it.
Are you ok with delete then ?
But I can obviously leave it as it was, the priority being to fix that OPTIONS issue.
There was a problem hiding this comment.
delete is usually slower than just creating a new Object.
Maybe just fix the OPTIONS and keep that code and just open a new issue to discuss it.
Fixes #82, with a new test reflecting that.
Note that the method to remove the
content-lengthheader is slightly changed:hasOwnPropertyis probably betterdeleteis more straightforward than usingfilterHeaders().By the way, that function was only used here but I leaved it in lib/util.js, in case you would like to keep it.