c: use proper type coercion in consume

[indutny]

On 32bit platforms size_t is essentially uint32_t (or at times
even meager uint16_t). Loading uint64_t field value into size_t on
these platforms would truncate the high bits and leave only the low 32
(16) bits in place. This leads to various interesting errors in
downstream modules. See:

• download of compressed files on arm-plattforms llhttp#110
• Response body ends prematurely undici#803

This patch makes all field loads go into their respective types.
Truncation doesn't happen in this case because C coercion rules will
cast both types to the largest necessary datatype to hold either of
them.

cc @Trott @mcollina @nodejs/http

I'd appreciate if we could fast-track this. We've been avoiding issues with this for awhile, but both wasm build in unidici and some arm users from the llhttp#110 hit this consistently on large uploads/downloads.

[indutny]

cc @addaleax as well since you are so good at C/C++!

[targos]

Does it affect 32-bit builds of Node.js too?

[indutny]

Does it affect 32-bit builds of Node.js too?

I believe it does.

[mcollina]

lgtm

[mcollina]

+1 for fast-tracking this.

[indutny]

FWIW, here's how the generated code looks with this patch:

      size_t avail;
      uint64_t need;
      
      avail = endp - p;
      need = state->content_length;
      if (avail >= need) {
        p += need;
        state->content_length = 0;
        goto s_n_llhttp__internal__n_span_end_llhttp__on_body;
      }
      
      state->content_length -= avail;
      return s_n_llhttp__internal__n_consume_content_length;
      /* UNREACHABLE */;
      abort();

Note the uint64_t above. It was size_t for both variables before the patch.

[indutny]

Landed in 037dd2f. Thanks y'all!

[Chaphasilor]

Landed in 037dd2f. Thanks y'all!

Hey @indutny thanks for this fix! Could you share any info on how soon this will trickle down into actual Node, and on which versions it will work?
Or is it just a matter of rebuilding Node and it will use the latest version of llparse? :)

[indutny]

I'm still waiting for llhttp's CI run to complete, and will publish patch version releases after that. Will submit PRs to Node.js right after that. We should be able to have a discussion on the release timing there!