Fix: error handling for Variable Byte Integer #95

[twegener-embertec]

This fixes the regression outlined in #95 which was introduced by #90 . See #95 for more specific details of the primary problem being addressed here.

Changes:

• Avoid emitting false errors when not all bytes are present yet when parsing the variable byte integer for the length header.
• Emit error upon attempting to generate a variable byte integer beyond the maximum allowed size.
• Add more test coverage in this area.

[mcollina]

@YoDaMa could you take a look as well?

[YoDaMa]

can you squash your commits down into 1 or 2 commits then I'll approve and merge @twegener-embertec

[twegener-embertec]

@YoDaMa can you please explain to me your rationale / guidelines / philosophy for squashing the commits?

I deliberately kept those commits separate since the purpose/effects of each are distinct:

• Add additional test coverage that still passes against the old code.
• Add test that exposes one pre-existing bug.
• Add test that exposes another pre-existing bug.
• Provide the fix.
• Restructure the test to deal with some tangentially related nodejs compatibility issue.

I'm reluctant to squash them as that loses that information, and also makes it harder to rewind and demonstrate the historical bugginess aspects.

I don't currently see any benefits in squashing them.

Perhaps you could do a --no-ff merge if you want the overall change to be obvious as well?

[YoDaMa]

@YoDaMa can you please explain to me your rationale / guidelines / philosophy for squashing the commits?

I deliberately kept those commits separate since the purpose/effects of each are distinct:

fair. I like that logic. The last commit seems extraneous beyond the earlier 5 commits. But it's good enough.

[twegener-embertec]

The last commit seems extraneous beyond the earlier 5 commits. But it's good enough.

Assuming you mean 909cbd3626, that was added to give evidence in response to your concerns about the maxBytes threshold. Ideally, I suppose that that commit should be reordered prior to the fix commit, so that it demonstrates that other bug before the fix.

If you want I could do that reordering via an interactive rebase, but I'd want to do that on a new branch to avoid screwing with this already public branch. I'm not sure how to integrate such a revised branch into this pull request, so if you want that please advise how. Otherwise I'll assume the "it's good enough" still stands.

[psorowka]

@mcollina is this already released and included in a current mqttjs release?