Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sending response.body using formdata-node fails due to Buffer reference #1718

Open
4miners opened this issue Feb 23, 2023 · 1 comment · May be fixed by #1719
Open

Sending response.body using formdata-node fails due to Buffer reference #1718

4miners opened this issue Feb 23, 2023 · 1 comment · May be fixed by #1719
Labels
bug

Comments

@4miners
Copy link

4miners commented Feb 23, 2023

I experienced the same issue as described here - #1413.

TypeError: Cannot perform Construct on a detached ArrayBuffer
    at new Uint8Array (<anonymous>)
    at new FastBuffer (node:internal/buffer:959:5)
    at Buffer.slice (node:buffer:1121:10)
    at TLSSocket.onData (file:///D:/www/upload-test/node_modules/node-fetch/src/index.js:407:36)
    at TLSSocket.emit (node:events:538:35)
    at addChunk (node:internal/streams/readable:315:12)
    at readableAddChunk (node:internal/streams/readable:289:9)
    at TLSSocket.Readable.push (node:internal/streams/readable:228:10)
    at TLSWrap.onStreamRead (node:internal/stream_base_commons:190:23)

Reproduction

I'm using following code to reproduce the error:

import { FormData } from 'formdata-node';
import { createServer } from 'http';
import fetch from 'node-fetch';

class BlobFromStream {
    #stream;

    constructor(stream, size) {
        this.#stream = stream;
        this.size = size;
    }

    stream() {
        return this.#stream;
    }

    get [Symbol.toStringTag]() {
        return 'Blob';
    }
}

createServer((req, res) => {
    res.end(Buffer.alloc(64 * 1024 * 10, '_'));
}).listen(3000);

const response = await fetch('http://localhost:3000');
const form = new FormData();

const contentLength = Number(response.headers.get('Content-Length'));

form.append(
    'file',
    new BlobFromStream(response.body, Number(contentLength)),
    `index.html`
);

await fetch('http://localhost:3000', {
    method: 'POST',
    body: form
});
process.exit(0);

I tracked down the issue to the following function:

node-fetch/src/index.js

Lines 395 to 407 in 8ced5b9

const onData = buf => {
properLastChunkReceived = Buffer.compare(buf.slice(-5), LAST_CHUNK) === 0;
// Sometimes final 0-length chunk and end of message code are in separate packets
if (!properLastChunkReceived && previousChunk) {
properLastChunkReceived = (
Buffer.compare(previousChunk.slice(-3), LAST_CHUNK.slice(0, 3)) === 0 &&
Buffer.compare(buf.slice(-2), LAST_CHUNK.slice(3)) === 0
);
}
previousChunk = buf;
};

I modified the function to add some debugging:

const onData = buf => {
	console.log(`previousChunk: ${previousChunk && previousChunk.byteLength}`);
	console.log(`Buffer: ${buf && buf.byteLength}`);

	properLastChunkReceived = Buffer.compare(buf.slice(-5), LAST_CHUNK) === 0;

	// Sometimes final 0-length chunk and end of message code are in separate packets
	if (!properLastChunkReceived && previousChunk) {
		try {
			properLastChunkReceived = (
				Buffer.compare(previousChunk.slice(-3), LAST_CHUNK.slice(0, 3)) === 0 &&
				Buffer.compare(buf.slice(-2), LAST_CHUNK.slice(3)) === 0
			);
		} catch (error) {
			console.log(error.message)
		}
	}

	previousChunk = buf;
};

The result is:

fixResponseChunkedTransferBadEnding called...
previousChunk: undefined
Buffer: 65536
fixResponseChunkedTransferBadEnding called...
previousChunk: 0
Buffer: 65536
Cannot perform Construct on a detached ArrayBuffer
previousChunk: 0
Buffer: 65536
Cannot perform Construct on a detached ArrayBuffer
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 0
Buffer: 65536
Cannot perform Construct on a detached ArrayBuffer
previousChunk: 0
Buffer: 65536
Cannot perform Construct on a detached ArrayBuffer
previousChunk: 0
Buffer: 65536
Cannot perform Construct on a detached ArrayBuffer
previousChunk: 0
Buffer: 65536
Cannot perform Construct on a detached ArrayBuffer
previousChunk: 0
Buffer: 99
Cannot perform Construct on a detached ArrayBuffer
previousChunk: undefined
Buffer: 65536

Looks like previousChunk variable is not holding the actual previous buffer. Notice that previousChunk behavior is not consistent, sometimes it contains the buffer sometimes it's empty. Probably because is set by reference and in the meantime, the buffer is drained in some other place.

Expected behavior

  • There should be no Cannot perform Construct on a detached ArrayBuffer error.
  • previousChunk variable from onData function to be reliable should always contain the actual previous buffer.

Potential solution
Modify the following line to copy the buffer instead of passing the reference:

node-fetch/src/index.js

Line 406 in 8ced5b9

previousChunk = buf; 

previousChunk = Buffer.from(buf);

or

previousChunk = new Buffer.alloc(buf.byteLength);

for (var i = 0; i < buf.length; i++)
    previousChunk[i] = buf[i];

The result now:

fixResponseChunkedTransferBadEnding called...
previousChunk: undefined
Buffer: 65536
fixResponseChunkedTransferBadEnding called...
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 99
previousChunk: undefined
Buffer: 65536

Your Environment

software version
node-fetch 3.3.0
formdata-node 5.0.0
node 16.14.2
npm 9.1.1
Operating System Widnows 10 22H2 (19045.2604)

Additional context

However, if I use form-data package there is no issue. So I'm not entirely sure if the issue is with fetch-node itself, it can be related to how formdata-node handle buffers.

import FormData from 'form-data';
import { createServer } from 'http';
import fetch from 'node-fetch';

createServer((req, res) => {
    res.end(Buffer.alloc(64 * 1024 * 10, '_'));
}).listen(3000);

const response = await fetch('http://localhost:3000');
const form = new FormData();

const contentLength = Number(response.headers.get('Content-Length'));

form.append('file', response.body, { filename: `index.html` });

await fetch('http://localhost:3000', {
    method: 'POST',
    body: form
});
process.exit(0);

Result:

fixResponseChunkedTransferBadEnding called...
previousChunk: undefined
Buffer: 65536
fixResponseChunkedTransferBadEnding called...
(node:17344) [https://github.com/node-fetch/node-fetch/issues/1167] DeprecationWarning: form-data doesn't follow the spec and requires special treatment. Use alternative package
(Use `node --trace-deprecation ...` to show where the warning was created)
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 65536
previousChunk: 65536
Buffer: 99
previousChunk: undefined
Buffer: 65536
@4miners 4miners added the bug label Feb 23, 2023
@4miners 4miners linked a pull request Feb 24, 2023 that will close this issue
Open
1 task
@lorenzopolidori
Copy link

@4miners I am experience exactly the same issue. What is the timeline for this PR to be merged and this fix being released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
2 participants
@lorenzopolidori @4miners