http.get closes before end of data from v1.97

[sirpy]

on ESP8266
I'm trying to download a 4k file via http.get
on v1.96 it works just fine, the res.on(data) is called multiple times and the whole file is downloaded
from v1.97-v1.99 res.on(data) is called only once with around 400bytes and then res.on(close) is called, so the file is not downloaded correctly.

[gfwilliams]

@opichals ? Could this be related to the HTTP chunked stuff again?

[wilberforce]

@sirpy

Please can you provide a simple test case?

Is the esp8266 only or does this occur on the Linux build too?

[opichals]

@gfwilliams Could be.
@sirpy Please provide a test case.

There is test_http_post_chunked.js and other test_http*.js but perhaps that is not sufficient.

[sirpy]

seems like it only happens on my nodejs server, it doesnt happen with local apache server.
The only thing i found different is that nodejs/expressjs uses lowercase response headers.
so if the http client checks for case sensitive headers it might be the issue.

[opichals]

@sirpy Yeah, the HTTP headers letter case handling is not addressed.

Perhaps we could convert to lowercase in the socketserver.c. It would break existing header handling code though. An alternative lowercase headers lookup object could be added and later we could deprecate the current exact casing support.

I'd be nice to rather have a special Object type for the case-insensitive key lookup. @gfwilliams ?

[gfwilliams]

@sirpy please could you hack up some simple node.js server code that lets us reproduce the issue here?

@opichals it seems that Node.js does lowercase them so I guess it'd be more compliant - however I can pretty much guarantee I'll get complaints if we mess with the case of the headers that get reported back - there'll be a bunch of people's code out there that relies on it (I can see at least 3 things in EspruinoDocs, one of which is the WebSockets lib).

I guess the easiest solution for now is to make a jsvObjectGetChildIgnoreCase that does a case-insensitive search, and then to use that? I'm happy to do that but I'd really like to have a way of testing that I've fixed the issue first.

I'm still a bit confused about the HTTP Chunking - it never seemed to be an issue before, but when the client started reporting HTTP/1.1 did some servers start using it in their responses?

[gfwilliams]

... having said that about headers, maybe it's better to do it now rather than later :) I guess it's not too bad to change the code that uses it to headers["Content-Type"]||headers["content-type"]

[opichals]

I'm sure we could find multiple use-cases for a case-insensitive Object lookup. I'd rather do that and allow users to use that from their JS code too (instead of doing the jsvObjectGetChildIgnoreCase).

[gfwilliams]

instead of doing the jsvObjectGetChildIgnoreCase

Not sure I understand?

I'm sure we could find multiple use-cases for a case-insensitive Object lookup

Amazingly this doesn't seem to be part of basic JS. I guess we could expose it via an E.lookupNoCase sort of function?

[opichals]

E.lookupNoCase sounds great!

[gfwilliams]

@sirpy please can you try the latest travis builds out and see if this fixes your issue

[sirpy]

i will... tonight. Thanks!

…

On Wed, Jun 6, 2018 at 4:50 PM, Gordon Williams ***@***.***> wrote: @sirpy <https://github.com/sirpy> please can you try the latest travis builds out and see if this fixes your issue — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1458 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAo9d9jL2lV_NDgmBZobkORq6C0-yBTtks5t5944gaJpZM4UaiUJ> .

-- Hadar Rottenberg https://www.linkedin.com/in/hadarrottenberg/ https://dablock.co https://medium.com/dablock https://meetup.com/dablock telegram <https://t.me/joinchat/FV4L_lLidGNQsMnY7jGCQA> +972-50-7319093

[opichals]

@gfwilliams I'd be nice to make the http request/response headers objects actually be a Proxy (just added to the ES* wishlist) as follows by default:

let headers = {
  'Content-Type': 'text/plain'
};
headers = new Proxy(headers, { get: E.lookupNoCase });

console.log(headers['content-type']);
// output: "text/plain"
console.log(Object.keys(headers));
// output: [ "Content-Type" ]

This way the node.js targetted code would just work in Espruino as the lowercase lookups would be ok.