Transfer-Encoding: chunked parser - JavaScript

Question

This code parses a series of Uint8Arrays that comprise a Transfer-Encoding: chunked request

function getChunkedData(u8) {
 let crlfIndex = 0;
 let chunkLength = "";
 let chunkBuffer = new Uint8Array(0);
 crlfIndex = u8.findIndex((v, k, array) => v === 13 && array[k + 1] === 10);
 chunkLength = parseInt(
 String.fromCodePoint(...u8.subarray(0, crlfIndex)),
 16,
 );
 chunkBuffer = u8.subarray(crlfIndex + 2, crlfIndex + chunkLength + 2);
 crlfIndex += chunkLength + 4;
 if (isNaN(crlfIndex)) {
 console.log({
 crlfIndex,
 chunkLength,
 chunkBuffer,
 inputBufferLength: u8.length,
 });
 }
 return {
 crlfIndex,
 chunkLength,
 chunkBuffer,
 inputBufferLength: u8.length,
 };
}

Server usage

if (!/(GET|POST|HEAD|OPTIONS|QUERY)/i.test(request) && !this.ws) {
 if (pendingChunkLength) {
 let rest = r.subarray(0, pendingChunkLength);
 len += rest.length;
 console.log(rest, len);
 let {
 crlfIndex,
 chunkLength,
 chunkBuffer,
 inputBufferLength,
 } = getChunkedData(r.subarray(pendingChunkLength - 1));
 if (chunkBuffer.length) {
 len += chunkBuffer.length;
 console.log(chunkBuffer, len, inputBufferLength, crlfIndex);
 }
 pendingChunkLength = 0;
 return;
 }
 let {
 crlfIndex,
 chunkLength,
 chunkBuffer,
 inputBufferLength,
 } = getChunkedData(r);
 len += chunkBuffer.length;
 console.log(chunkBuffer, len);
 if (chunkBuffer.length < chunkLength) {
 pendingChunkLength = chunkLength - chunkBuffer.length;
 }
}

Client usage

var abortable = new AbortController();
var {
 readable,
 writable
} = new TransformStream({
 async transform(v, c) {
 for (let i = 0; i < v.length; i += 4096) {
 c.enqueue(v.subarray(i, i + 4096));
 await scheduler.postTask(() => {}, {
 delay: 20
 });
 }
 },
 flush() {
 console.log("flush");
 abortable.abort("");
 }
}, {
 highWaterMark: 1
});
var writer = writable.getWriter();
var response = fetch("http://localhost:44818", {
 method: "post",
 duplex: "half",
 body: readable,
 signal: abortable.signal,
 allowHTTP1ForStreamingUpload: true
});
response.then((r) => {
 console.log(...r.headers);
 return r.body.pipeTo(
 new WritableStream({
 write(v) {
 console.log(v);
 },
 close() {
 console.log("close");
 }
 })
 )
 })
 .then(() => {
 console.log("Done streaming");
 })
 .catch(console.log);
await scheduler.postTask(() => {}, {
 delay: 10
});
await writer.write(new Uint8Array(1024 ** 2));
await writer.ready;
await writer.close();

Answer 1

This question shows the same problem as the code: there seems to be an expectation that we understand as much as the author. No actual question is being stated, and we don't know what this code is supposed to do. The only thing we can establish is that it is trying to parse chunks using JS - so much is in the title, but that's about it.

This code is simply not good enough, and it sorely needs documentation and inline comments to explain why things are happening.

---

To compare to CR/LF it is probably better to use constants that are more clearly specified, e.g.:

const CR = '\r'.charCodeAt(0); // the 13 in the code
const LF = '\n'.charCodeAt(0); // the 10 in the code

---

let crlfIndex = 0; 

OK, so I suppose this is going to be used to find a Windows style line ending, other line endings are apparently banned. The crlfIndex is assigned a value, but the next code will immediately replace that value.

---

 let chunkLength = "";

Not only is this not necessary, but it receives a value from parseInt later.

---

crlfIndex = u8.findIndex((v, k, array) => v === 13 && array[k + 1] === 10);

Immediately we're thrown off course with 13 and 10, two magic variables that are become even more magic because there is no indication whatsoever what they may be. Here constants should be used. At least the hexadecimal values make sense to more users (0x0D and 0x0A).

---

String.fromCodePoint(...u8.subarray(0, crlfIndex)),

Since you are only parsing characters without any error checking, it may be better to simply assume ASCII?

---

 crlfIndex += chunkLength + 4;
 if (isNaN(crlfIndex)) {

Do not use a check on a value after += with a number, JS can use coercion. So you would probably get a "true" even if crlfIndex is not a number before the addition using += - and of course possibly a wrong number.

---

 console.log({
 crlfIndex,
 chunkLength,
 chunkBuffer,
 inputBufferLength: u8.length,
 });
 }
 return {

After finding an issue with the input, it is important not to just log and continue. Normally an exception or Error is generated instead. This code simply asks for illegal state, and generally one gets what is asked for.

---

 } = getChunkedData(r.subarray(pendingChunkLength - 1));

Why minus one? Did I miss anything - apart of course from any explanation?

---

 pendingChunkLength = 0;
 return;

Wait, there was a pending chunk, which is then removed by a single read? So we have two chunks maximum? At this point I'm merely guessing to be honest, as I don't get the code.

---

 for (let i = 0; i < v.length; i += 4096) {
 c.enqueue(v.subarray(i, i + 4096));

Create a constant, especially if you're going to use it twice.

• guest271314: If you don't understand what's going on, maybe you shouldn't have answered the question? I'm parsing Transfer-Encoding: chunked data that is read in the server as a series of Uint8Arrays, inside Chromium browser. The server is TCPServerSocket. Here's the specification datatracker.ietf.org/doc/html/rfc9112#name-transfer-encoding. The question has nothing to do with Windows.
• Maarten Bodewes: HTTP and telnet use the same line endings as Windows; this has grown like that historically. However, I'm indicating that if you receive chunks of data then other line endings may be in there as well. I'm not sure what you are trying to accomplish. Maybe this is a way for you to read out the headers of a HTTP request. Could well be the case, don't know why you should chunk that or why you would look for that as you haven't explained. But hey, I guess you're golden, as you seem to be understanding what you're doing, right? That's all a program really needs.
• guest271314: I explained what is going on in OP. In prose and in code. I'm making a POST request using fetch() with a ReadableStream set as body. Fetch over HTTP/1.1, on Chromium based browsers, use Trasnfer-Encoding: chunked, when the feature flag is set to allow upload streaming over HTTP/1.1. The server is also Chromium browser, inside an Isolated Web App where Direct Sockets TCPServerSocket is defined. When the client writes to the WritableStreamDefaultWriter, one or more Uint8Arrays are received in the server, with length encoded, then the data per the linked specification.
• J_H: "maybe you shouldn't have answered the question?" Maarten was volunteering some valuable feedback about whether the OP code is maintainable, as at least one reviewer found it a bit opaque. When we do a Pull Request on a feature branch, prior to merging it to main, we're trying to answer two questions about the code. (1.) Is it correct? (2.) Is it maintainable? That 2nd item gets at whether team members, beyond the author, could fix bugs and add features. It addresses whether a project can recruit new contributors as circumstances change. The feedback on (2.) was "nope, not yet." I agree.
• Mast: @guest271314 Please keep this conversation civilized. The alternative is throwing out all comments on this answer, which serves no one. The will of reviewers and the scope of a review is something you can try to discuss on meta (it may have been discussed before), not below answers. Answers are written by volunteers. If they contain information you don't want to use, don't use it. That doesn't make the information wrong.