Moddable-OpenSource / moddable

Tools for developers to create truly open IoT products using standard JavaScript on low cost microcontrollers.
http://www.moddable.com
1.32k stars 236 forks source link

embedded:network/http/client requires headers in lower case #1218

Closed tve closed 11 months ago

tve commented 11 months ago

Build environment: Linux Moddable SDK version: idf-v5 branch Target device: esp32-c3

Description Looking at https://419.ecma-international.org/#-20-http-client-class-pattern I do not see the requirement that the headers passed to the request method be all lower case. I passed ["Content-Length": ...] and this caused no onWritable callback due to the lower-case equality check in https://github.com/Moddable-OpenSource/moddable/blob/public/examples/io/tcp/httpclient/httpclient.js#L343 . Note the same issue for transfer-encoding.

Expected behavior I expected a case-insensitive comparison or a sentence in the spec to state that the headers must be all lower-case. (I bet the latter is there somewhere and I just haven't seen it....)

phoddie commented 11 months ago

The implementation certainly expects lowercase header names on input and generates lowercase headers names on output. When used with Header as the fetch() implementation does, this happens automatically. The spec doesn't say anything about the input headers being lowercase though. The fix is easy, as I'm sure you found. Add the following before line 343:

name = name.toLowerCase();

This allows the script to control the case of the headers sent, in case it matters (it shouldn't).. Alternatively, the toLowerCase() could be done in the assignment in line 341.

I'll commit that so no one else has to trip over this.

Using Map, there is the messy case where it can contain "Content-Length", "content-length", "CONTENT-LENGTH", and more. There's no point in handling that, so I think the spec should just note it as an undefined behavior.

What do you think?

phoddie commented 11 months ago

Also.... the HTTP 1.1 specification states:

All transfer-coding names are case-insensitive...

I think that means that the chunked check needs to be updated as well (not that anyone uses "CHUNKED"):

if ("chunked" === item.value[1].toLowerCase())
phoddie commented 11 months ago

Fix committed. Closing. (If problem persists, please reopen.)