httpolyglot icon indicating copy to clipboard operation
httpolyglot copied to clipboard

Peeked first byte is lost from parser data

Open pimterry opened this issue 5 years ago • 4 comments

I'm trying to use httpolyglot and also listen to subsequent client errors using server.on('clientError', ...).

The listener for this event is passed the error, with the raw packet data from the parser attached as error.rawPacket.

When I receive this data, for plain HTTP connections that have been passed through httpolyglot, the first byte is missing from the raw packet, so that GET / HTTP/1.1 becomes ET / HTTP/1.1. This doesn't happen for HTTPS connections, only plain HTTP.

I'm trying to log the packet data from these errors for debugging, so this is a little annoying! I'd love to be able to avoid this. It seems like the socket.unshift here isn't sufficient to replace the peeked data exactly as it was, presumably due to how the HTTP parser handles that buffer. Any ideas on how I could fix that?

pimterry avatar Apr 14 '20 11:04 pimterry

No idea offhand. This repo was meant more as a POC than anything. A lot has changed internally in node over the course of 3+ years, so I would've been surprised if the code in this repo hadn't been affected in that span of time.

With that in mind, I'm open to PRs to fix issues anyone may find, but at this time I am not actively keeping track of breakage with different node versions.

mscdex avatar Apr 14 '20 20:04 mscdex

Fair enough! Thanks for letting me know anyway.

For now, I've hacked together a workaround that at least exposes the peeked data, so I can grab it later: https://github.com/httptoolkit/httpolyglot/commit/ac0a75fbca4083c15cb9d7386e441087562548cd. Not a properly usable fix though imo (you need to know that when you need that extra peeked data, and then prepend it to the rest of your data) so I don't think it's a sensible change to httpolyglot in general. It has solved my very immediate problem anyway. I'll open a PR with a proper fix if I manage to find something that solves this properly.

pimterry avatar Apr 15 '20 14:04 pimterry

In which version of nodejs does this problem occur? Is there a reproducible code base for __httpPeekedData? I used nodejs v13 and did not encounter this problem,

masx200 avatar Apr 21 '20 11:04 masx200

https://stackoverflow.com/a/23975955

wll8 avatar Nov 14 '22 03:11 wll8