Bug in mochiweb_request:read_chunk_length/2

davisp commented 7 years ago

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.

etrepum commented 7 years ago

Do you mind submitting a PR that fixes this issue, since you have everything already there to test & reproduce it?

davisp commented 7 years ago

Yeah, sorry. Meant to make a note that I'd get to it. Just wanted to write things down before I moved on and forgot about it.

On Thu, Jun 8, 2017 at 5:59 AM Bob Ippolito notifications@github.com wrote:

Do you mind submitting a PR that fixes this issue, since you have everything already there to test & reproduce it?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mochi/mochiweb/issues/188#issuecomment-307070699, or mute the thread https://github.com/notifications/unsubscribe-auth/AABN2c6VjQRz_uFSSuaTTBn_ovRkVqAnks5sB9P0gaJpZM4NwkND .