nread var incorrect (off by 1) when header spans multiple packets

[HAT-DK]

Found in version 2.8.1.
If a HTTP header spans 2 IP packets the nread state var is off by one.
The bug is caused since the nread var is incremented before a pointer check is done (which decrements the pointer value by 1):
http_parser.c:1637

        COUNT_HEADER_SIZE(p - start);

        if (p == data + len)
          --p;

should have been:

        if (p == data + len)
          --p;

        COUNT_HEADER_SIZE(p - start);

The same issue seems to reside in http_parser:1342

The issue is easily reproduced by injecting part of an HTTP request and verifying the nread var after http_parser_execute: "GET / HTTP/1.1\r\nHost: myhost".

[bnoordhuis]

Confirmed, thanks. Fix in #427.

[ploxiln]

correct me if I'm wrong: is nread only used for checking against HTTP_MAX_HEADER_SIZE?

[bnoordhuis]

In http_parser itself? Yes.

I know there are some downstream users that use it for whatever reason. It's not part of the public API but that never stopped C programmers before.

[HAT-DK]

nread can also be used (uofficially) to track the size of the headers,
@bnoordhuis Thanks a lot for the super speedy response. Any guess as to how long before it's merged to the master branch??

[bnoordhuis]

I asked @indutny to review it. If he's okay with it, I can merge it and do a 2.8.2 release.

[HAT-DK]

I've also noticed that the nread is sometimes incorrect when starting to parse a new message (off by 2).
This seems to be caused since the nread var is NOT reset on message_complete, and sometimes have some spill-over from previous msg parsing. I see this in responses with chunked-content.
It would be super useful, for me at least, if the nread is ALWAYS reset on message_complete.
Should I create a new issue for this?

[bnoordhuis]

That'd be best. There may be some backwards compatibility issues to discuss and that's best done in its own issue.