throw when there's no boundary

[gurgunday]

This PR fixes #121

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[gurgunday]

Latest bench:

{ cpu: { brand: 'M1', speed: '2.4 GHz' } }
| Node    | Option         | Msecs/op       | Ops/sec  | V8                    |
| ------- | -------------- | -------------- | -------- | --------------------- |
| 20.5.0  | fastify-busboy | 0.113441 msecs | 8815.123 | V8 11.3.244.8-node.10 |
| 20.5.0  | busboy         | 0.206708 msecs | 4837.743 | V8 11.3.244.8-node.10 |
| 16.13.0 | fastify-busboy | 0.270984 msecs | 3690.248 | V8 9.4.146.19-node.13 |
| 16.13.0 | busboy         | 0.340114 msecs | 2940.190 | V8 9.4.146.19-node.13 |
| 12.22.7 | busboy         | 1.945927 msecs | 513.894  | V8 7.8.279.23-node.56 |

[Uzlopak]

Final remark

[Uzlopak]

It seems that the unit tests dont fail in any case. If bufferStartsWith always returns false or always returns true, npm test runs pass always.

[gurgunday]

It seems that the unit tests dont fail in any case. If bufferStartsWith always returns false or always returns true, npm test runs pass always.

They pass because if the function returns true, no error is thrown, and if false, shouldError catches that error and makes the test pass. You could remove shouldError to see them fail:

// types-multipart.spec.js
    {
      source: [
        '    ------WebKitFormBoundaryTB2MiQ36fnSJlrhY--\r\n'
      ],
      boundary: '----WebKitFormBoundaryTB2MiQ36fnSJlrhY',
      expected: [],
      shouldError: 'Unexpected end of multipart data',
      what: 'empty form with preceding whitespace'
    },
    {
      source: [
        '------WebKitFormBoundaryTB2MiQ36fnSJlrhY--\r\n'
      ],
      boundary: '----WebKitFormBoundaryTB2MiQ36fnSJlrhYY',
      expected: [],
      shouldError: 'Unexpected end of multipart data',
      what: 'empty form with wrong boundary (extra Y)'
    },
    {
      source: [
        ''
      ],
      boundary: '----WebKitFormBoundaryTB2MiQ36fnSJlrhY',
      expected: [],
      shouldError: 'Unexpected end of multipart data',
      what: 'No fields and no files'
    },

[Uzlopak]

The test should not pass if it is intended to fail. I am currently working on #123 to replace mocha/chai with tap. Maybe first transform to tap?

[Uzlopak]

No test is better than having a test that never fails. ;)

[gurgunday]

Hmm, this one should fail if the function always returns false though but it doesn't – it used to a few commits ago:

    {
      source: [
        '------WebKitFormBoundaryTB2MiQ36fnSJlrhY--\r\n'
      ],
      boundary: '----WebKitFormBoundaryTB2MiQ36fnSJlrhY',
      expected: [],
      what: 'empty form'
    },

[Uzlopak]

#123 can be reviewed. Have a look please

[gurgunday]

Hey, @Uzlopak, could you please see the latest commit, it simply calls end on dicer and lets it handle the input

Tests seem to all pass nicely, let me know if something doesn't add up

[Uzlopak]

Could you please explain further, what the issue was?

[gurgunday]

Could you please explain further, what the issue was?

There was a lot of duplication going on as Dicer also handles validation, but we would skip it by directly emitting finish when ‘nparts’ was 0

I haven’t benchmarked which approach is faster but dicer has already finished parsing the input by the time we reach the end and throws when the boundary does not exist

So I couldn’t be sure if adding all that code was necessary

[Uzlopak]

LGTM

[Uzlopak]

@kibertoad
Please also take a peek

[Uzlopak]

marking as semver-major, as it changes the behaviour significantly.