'Unexpected end of form' when transfer-encoding: chunked

[wickedest]

I encountered a breaking change on 1.0.0 of busboy. One of my unit-tests failed. It sends am empty form with "transfer-encoding: chunked", e.g. the raw HTTP request:

POST / HTTP/1.1
Host: localhost:9090
User-Agent: curl/7.58.0
Accept: */*
transfer-encoding: chunked
Content-Type: multipart/form-data; boundary=---------BOUNDARY

0

busboy@0.3.1 correctly handles the request. busboy@1.0.0 errors with 'Unexpected end of form'

index.js

const http = require('http');
const busboy = require('busboy');

http
    .createServer((req, res) => {
        if (req.method !== 'POST') {
            return res.status(404).send();
        }
        const bb = busboy({ headers: req.headers });
        bb.on('file', (name, file, info) => {
            console.log('file');
            file.on('data', (data) => {
                console.log('data:', data.length);
            });
        });
        bb.on('field', (name, val, info) => {
            console.log('field');
        });
        bb.on('close', () => {
            console.log('closed');
            res
                .writeHead(303, { Connection: 'close', Location: '/' })
                .end();
        });
        bb.on('error', (err) => {
            console.log('error', err);
        });
        bb.on('finish', () => {
            console.log('finished');
        });
        req.pipe(bb);
    })
    .listen(9090, () => {
        console.log('Listening for requests');
    });

To reproduce:

curl -X POST http://localhost:9090\
 --header 'transfer-encoding: chunked'\
 --header 'Content-Type: multipart/form-data; boundary=---------BOUNDARY'\
 --data-binary ''

Also, the busboy README.md does not mention anything about handling error events. This example will crash the server if the 'error' event isn't registered. It might be a good idea to update the example.

[mscdex]

The new behavior is more correct in my opinion. If the request doesn't even include the end boundary marker, that doesn't seem right. Seeing as how this was introduced in a new major version, breaking changes are to be expected.

As far as 'error' events go, they are standard, special events for any streams in node (any EventEmitters actually), so it doesn't make sense to duplicate such documentation. Writable stream documentation can be found here.

[wickedest]

@mscdex, are you sure? seems like a bug to me. it doesn't contain a start or end boundary marker. the chunk is 0. this would be the equivalent of a form where all fields are optional, e.g. equivalent to an empty form with content-length: 0. For example:

PUT / HTTP/1.1
Host: localhost:9090
User-Agent: curl/7.58.0
Accept: */*
Content-Type: multipart/form-data; boundary=---------BOUNDARY
Content-Length: 0

The only difference here is content-length is omitted for content-encoding: chunked.

Actually, content-length: 0 also fails with "Unexpected end of form". So it appears the issue is actually that clients can't send empty multipart forms.

Seeing as how this was introduced in a new major version, breaking changes are to be expected.

I looked for a list of documented breaking changes, but couldn't find any.

As far as 'error' events go, they are standard

Sure, but it would be a natural (and wise) thing to include 'error' in your very detailed example.

[mscdex]

If a multipart/form-data request without the closing boundary were to be accepted, there would be no way to distinguish it from a malformed request. Even browsers send at least the closing boundary for empty forms. If there are non-browser clients sending empty forms in this way, I would say they are broken.