if both isChunked and hasContentLength, set hasContentLength to false.

[bkbits]

// if both isChunked and hasContentLength, content length wins
// because it has been verified to match the body length already
// if both isChunked and hasContentLength, chunk wins
// Because Content-Length is the length of all chunk, when both isChunked and hasContentLength.
// The behavior is the same as chromium.
if (this.isChunked && hasContentLength) {
this.hasContentLength = false;
}

[Jimbly]

Did you mean hasContentLength = false? this.hasContentLength is not a thing.

Do you have a site or example this fixes? Can you verify this doesn't break the previous issue this change was added to fix (I think it should be fine...).

[bkbits]

Did you mean hasContentLength = false? this.hasContentLength is not a thing.

Do you have a site or example this fixes? Can you verify this doesn't break the previous issue this change was added to fix (I think it should be fine...).

This is screenshot on electron V3.0.4.

This is screenshot on chrome 69.x.


I Don't have a site, but I write a chunked example that both content length and chunked.

const http = require("http");
const fs = require("fs-extra-promise");

const test = "D:/test.txt";

function onError(err)
{
    if(err)
        console.error(err);
}

async function onRequestAsync(request, response) {
    let fileData = await fs.readFileAsync(test);
    let len = fileData.length;
    let chunkedSize = parseInt(len / 10);
    
    response.setHeader("Transfer-Encoding", "chunked");
    response.setHeader("Content-Type", "application/download");
    response.setHeader("Content-Disposition", "attachment;filename=test.txt");
    response.setHeader("Content-Length", len);
    response.flushHeaders();
    for(let i = 0; i < 9; ++i)
    {
        let chunk = fileData.subarray(i * chunkedSize, (i + 1) * chunkedSize);
        response.write(chunk.length.toString(16), onError);
        response.write(chunk, onError);
        response.flush();
    }

    let lastChunk = fileData.subarray(9 * chunkedSize);
    response.write(lastChunk.length.toString(16), onError);
    response.write("\r\n", onError);
    response.write(lastChunk, onError);
    response.write("\r\n", onError);   
    response.flush(); 
    response.write("0", onError);
    response.end("\r\n\r\n", onError);
}

function onRequest(request, response)
{
    onRequestAsync(request, response).then(()=>{
        console.log("finish.");
    }).catch((err)=>{
        console.error(err);
    });
}

http.createServer(onRequest).listen(8080);

[bkbits]

The pr is effective on node 8.x, but not on node 10.x. Maybe process.binding is deprecated.

[Jimbly]

Thanks for posting an example, I'll take a closer look this weekend or early next week.

process.binding should still be effective on Node 10 for the 'http_parser' binding, except when used in a child process/forked process, but might need to be changed to internalBinding instead in the future.

[bkbits]

Thanks for posting an example, I'll take a closer look this weekend or early next week.

process.binding should still be effective on Node 10 for the 'http_parser' binding, except when used in a child process/forked process, but might need to be changed to internalBinding instead in the future.

If I use webpack 4.x + electron 3.x and set HTTPParser, it will throw "HPE_INVALID_METHOD" on main process and it is not effective on renderer process. Why?

[Jimbly]

I don't know what those are, but if you're on Node 10.x, and it's something with child processes, it could be the Node 10.x issue I alluded to earlier (no longer able to override the process.binding of http_parser in child processes). For the "HPE_INVALID_METHOD", someone else reported getting that on Node 10.x, but never provided any details on how to reproduce. If you have a simple way to reproduce this and could add it to #57, that'd be great.

[Jimbly]

Okay, I think this seems good, wanted to also clear this.body_bytes back to null to get it to completely ignore content-length. Behavior is now the same as in Chrome. The parser can no longer parse responses that have these two headers but have a non-chunked body, however, Chrome cannot either, and I think that's probably the less-common case.

For my future reference, here's a super-simple server that serves the raw data of either case I used for testing:

const net = require('net');

let server = new net.Server();
server.listen(8081);
server.on('connection', function (s) {
	if (1) {
		// Gives correct 13-byte response on Chrome
		s.write([
			'HTTP/1.1 200 OK',
			'Transfer-Encoding: chunked',
			'Content-Type: text/plain',
			'Content-Length: 123',
			'Connection: keep-alive',
			'',
			'7',
			'thunk1\n',
			'6',
			'thunk2',
			'0',
			'',
			'',
	  ].join('\r\n'));
	} else {
		// Invalid chunked encoding on Chrome
		s.write([
			'HTTP/1.1 200 OK',
			'Transfer-Encoding: chunked',
			'Content-Type: text/plain',
			'Content-Length: 13',
			'Connection: keep-alive',
			'',
			'thunk1\nthunk2',
	  ].join('\r\n'));
	}
	s.end();
});

The earlier example was confusing because Node's HTTP module handles writing all of chunked headers automatically, so the example was writing some extra stuff (the terminating "0\r\n\r\n" which was also being written by Node).