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.

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

Same functionality as concat-stream #1

Closed
dmitrig01 opened this issue Mar 4, 2014 · 1 comment
Closed

Same functionality as concat-stream #1

dmitrig01 opened this issue Mar 4, 2014 · 1 comment

Comments

@dmitrig01
Copy link

What are the differences between this project and https://github.com/maxogden/concat-stream?

@jeffbski
Copy link
Owner

jeffbski commented Mar 5, 2014

They appear to be similar, however one of the original reasons for creating accum was to provide an easy way to accumulate a stream which might contain multibyte string which has been split across buffer packets in a stream.

The proper way to handle that is to concatenate all the buffers first, then convert to string, otherwise the conversion of individual packets will result in a different (corrupted) result string.

Since this was an easy mistake this module was created to make that and other accumulations easier.

Since I didn't have a test for that multibyte split, the original code had regressed over time, but after a review today, I added a test and restored that original use case.

From my review of concat-stream, it suffers from the same flaw (if a multibyte char is split between buffers, it will result with wrong corrupted string).

See multibyte char split test -858c22c

and the fix - 33d2961

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants