Bug in mochiweb_request:read_chunk_length/2
davisp opened this issue · 9 comments
I accidentally stumbled across this, but it looks like there's a ten year old bug when reading chunk lengths that doesn't properly skip trailing garbage after the hex encoded length.
https://github.com/mochi/mochiweb/blob/master/src/mochiweb_request.erl#L583
The intended logic for that is to split on any tab, newline, or space. However the original author or (at least someone before the large first commit in the repo) most likely had their editor set up to strip trailing whitespace which inadvertently removed the space after the last character escape. Which changes the logic to checking that C is not a tab, newline, or newline because Erlang will apparently happily apply the $ character escape to any whitespace character.
I only found this due to playing around with some source parsing tooling and this got caught up in one of my checks as an invalid construct. Granted its acceptable to Erlang so its my tool that's broken but I did realize it wasn't intentional in Mochiweb.
Do you mind submitting a PR that fixes this issue, since you have everything already there to test & reproduce it?
Should this issue be closed as it looks like the fix was incorporated?
It looks like the source was reformatted but the bug was not fixed 🤦♂
Here's the relevant RFC for how this is supposed to behave: https://tools.ietf.org/html/rfc7230#section-4.1
Thanks Bob. I'm new to mochiweb and was skimming the issues and mistakenly thought that particular one had been fixed.
Yes, this should have been closed.