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

Peeked first byte is lost from parser data #13

Open
pimterry opened this issue Apr 14, 2020 · 4 comments
Open

Peeked first byte is lost from parser data #13

pimterry opened this issue Apr 14, 2020 · 4 comments

Comments

@pimterry
Copy link

pimterry commented Apr 14, 2020

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?

@mscdex
Copy link
Owner

mscdex commented Apr 14, 2020

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.

@pimterry
Copy link
Author

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: httptoolkit/httpolyglot@ac0a75f. 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.

@masx200
Copy link

masx200 commented Apr 21, 2020

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,

@wll8
Copy link

wll8 commented Nov 14, 2022

https://stackoverflow.com/a/23975955

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants