-
-
Notifications
You must be signed in to change notification settings - Fork 754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
http.get closes before end of data from v1.97 #1458
Comments
@opichals ? Could this be related to the HTTP chunked stuff again? |
Please can you provide a simple test case? Is the esp8266 only or does this occur on the Linux build too? |
@gfwilliams Could be. There is test_http_post_chunked.js and other |
seems like it only happens on my nodejs server, it doesnt happen with local apache server. |
@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 ? |
@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 I'm still a bit confused about the HTTP Chunking - it never seemed to be an issue before, but when the client started reporting |
... 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 |
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 |
Not sure I understand?
Amazingly this doesn't seem to be part of basic JS. I guess we could expose it via an |
|
@sirpy please can you try the latest travis builds out and see if this fixes your issue |
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
|
@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. |
works great. thanks |
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.
The text was updated successfully, but these errors were encountered: