Skip to content
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

Fix receiving of large HTTP headers #1415

Merged

Conversation

@opichals
Copy link
Contributor

opichals commented May 12, 2018

Added a test which sends HTTP headers which take more than
MSS bytes (536) and therefore the httpParseHeaders() needs to be
called later again after the next packet arrives.

The websocket Connection: Upgrade request with the
Sec-WebSocket-* headers made the HTTP header section larger
as described above so it exhibited in the websocket scenario in #1405

@opichals opichals force-pushed the opichals:cope-with-http-headers-longer-than-mss branch from 48109e5 to 20a8bc1 May 12, 2018
@opichals opichals changed the title Fix large HTTP headers receive. Fix receiving of large HTTP headers May 12, 2018
@opichals opichals referenced this pull request May 12, 2018
Added a test which sends HTTP headers which take more than
MSS bytes (536) and therefore the httpParseHeaders() needs to be
called later again after the next packet arrives.

Fixes #1405
@opichals opichals force-pushed the opichals:cope-with-http-headers-longer-than-mss branch from 20a8bc1 to faf7157 May 12, 2018
@gfwilliams

This comment has been minimized.

Copy link
Member

gfwilliams commented May 14, 2018

Thanks! Yeah, on non-linux/esp* devices the data tends to come in as much smaller chunks, so you hit this pretty much right away.

@gfwilliams gfwilliams merged commit 056af74 into espruino:master May 14, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wilberforce

This comment has been minimized.

Copy link
Member

wilberforce commented May 14, 2018

@opichals

Any idea what the 536 mss size is based on?

@opichals

This comment has been minimized.

Copy link
Contributor Author

opichals commented May 14, 2018

@wilberforce I did some study there but lack complete understanding aside from the wikipedia MSS description at https://en.wikipedia.org/wiki/Maximum_segment_size

@gfwilliams

This comment has been minimized.

Copy link
Member

gfwilliams commented May 14, 2018

I think it was to do with the packet size ESP8266 usually tried to deal with? In Espruino it's just the size of chunks of data that are sent to/from the IP layer.

Raising it has reasonably minimal gains other than when transmitting, when the ESP8266/32 might choose to send a packet after every call to send - so that value would directly affect the transmission speed because you're limited in the amount of packets/second you can get through.

edit: basically 536 seemed to be a good toss-up between memory usage, overhead for Espruino, and wireless speed. Maybe the trade-off is different on ESP32 but you'd want to do a bunch of benchmarks before changing it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.