HTTPParserError: Expected HTTP

[ronag]

Got this error while running master on our test setup.

"HTTPParserError: Expected HTTP/\n    at Parser.[kMakeError] (/usr/src/app/node_modules/undici/lib/llhttp/parser.js:264:12)\n    at Parser.execute (/usr/src/app/node_modules/undici/lib/llhttp/parser.js:246:31)\n    at Socket.onSocketData (/usr/src/app/node_modules/undici/lib/client.js:793:17)\n    at Socket.emit (node:events:369:20)\n    at Socket.Readable.read (node:internal/streams/readable:524:10)\n    at Socket.read (node:net:622:10)\n    at flow (node:internal/streams/readable:1006:34)\n    at resume_ (node:internal/streams/readable:987:3)\n    at processTicksAndRejections (node:internal/process/task_queues:81:21)"

Source is a Node 15 server. So it's either an undici or node bug.

[ronag]

I suspect the WASM update. @dnlup any ideas?

[ronag]

pipelining is set to 1

[dnlup]

I suspect the WASM update. @dnlup any ideas?

Not atm, I'll dig into it. Do you have some payload examples?

[ronag]

Do you have some payload examples?

Sorry, not at the moment. I will also dig further as soon as I have time.

[dnlup]

Ok 😉 . The parser was starting to parse what it thought was a new message, and it didn't find HTTP. So it considered it malformed. Is there something particular about these requests? I did not implement any pause logic, which was present in the native Node parser. Could that mean something?

[ronag]

Is there something particular about these requests?

Not that I can think of... the error is gone now that I've reverted back to 3.x.

I did not implement any pause logic, which was present in the native Node parser. Could that mean something?

Maybe?

Sorry I can't be more helpful atm.

[dnlup]

the error is gone now that I've reverted back to 3.x.

It must be the parser then

[dnlup]

the error is gone now that I've reverted back to 3.x.

It must be the parser then

If you could checkout the commit that introduced the parser it would be a 100% proof

[ronag]

I need to try to reproduce it somewhere else. It wasn't the test setup it was another setup used in production and I can't do debugging there. So it might be a bit tricky / take a while. I'll also try to help reviewing the code and see if I can find anything obvious.

[ronag]

Would be nice if we had some way to do fuzzing both of request and response. 🤔 Any ideas? @mcollina do you have any experience with that?

[dnlup]

I am digging into Node tests to see if there is something relating to fuzzing. In the meantime I found this test:

* https://github.com/nodejs/node/blob/master/test/parallel/test-http-pause.js

Could that be a scenario that is happening in your environment?

Sorry, it does not test directly the parser.

[dnlup]

Sorry for spamming, I guess one thing I would look at is if in this part the buffer passed to execute is valid, and it does not change before passing it to the wasm realm. That would tell us if we are doing something wrong in execute.

[ronag]

@mcollina @dnlup I think this is a blocker for v4 release. I haven't been able to reproduce it outside of production system.

I guess our only choice is to do in depth review and see if we can find any path that could cause this.

Been looking at HTTP fuzzying but without much luck.

[mcollina]

I would hardly call it a bug until we can repro it :/. I would go ahead and release with the hope of finding more data points.

We need a reliable way to reproduce this.

[ronag]

I disagree. This error message should not be possible unless there is a bug somewhere. I’m not entirely comfortable doing a release before we’ve done a reasonable effort to at least try to resolve it.

[dnlup]

I am all up for finding the issue, but we need to reproduce it. I understand if this is a release blocker. Is there anything you could do in one of the environments that are affected? Like, could you manually change and debug the code? One first step for me would be trying this. My gut tells me it could be either something about how the buffer is handled or something about the parser missing a pause functionality.

As a note, maybe we could port the parser tests in Node here? Do you think they are applicable?

[mcollina]

fastify-reply-from has all sort of edge cases covered, I'll try to see if this shows up there.

Overall I would recommend @ronag to try to isolate a repro - so we can work on a fix.

[ronag]

I'll see if I can access the production system during some night this week when it's under less use and see if I can get more data points.

[mcollina]

I'll see if I can access the production system during some night this week when it's under less use and see if I can get more data points.

that would be fantastic. Try adding a few log lines to see what has lead to that situation. Ideally what is not HTTP.

[ronag]

I hope #687 solved this.