Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

Messages implicitly converted to string #542

Open
platy opened this issue Jul 14, 2016 · 4 comments
Open

Messages implicitly converted to string #542

platy opened this issue Jul 14, 2016 · 4 comments

Comments

@platy
Copy link

platy commented Jul 14, 2016

Background
I have just spent a lot of time debugging an issue of an array containing a single buffer being sent to ZMQ - in my test cases it worked but with production data there were extra bytes appearing all over the place.

Problem
The OutBuffer silently converts things which are not buffers into strings - link
The String conversion for arrays converts each element to string and concatenates them, the String conversion for Buffer treats the buffer as containing a UTF-8 encoded string.
So for most cases you don't notice a problem like this - ~60% of bytes are valid and survive.

Proposal
I don't think doing this conversion is worth the trouble it can cause - I think we should throw a TypeError - I will send a PR.

@platy
Copy link
Author

platy commented Jul 14, 2016

Sent PR #543

@ronkorving
Copy link
Collaborator

Hi @platy, I'm not sure I understand the problem. Are you saying sending a [new Buffer('foo')] did not work as expected? We traverse arrays if given, and the elements are then cast to string. The same is true for single arguments that are not arrays. I think you are describing this already, so we probably agree... but what's the actual problem?

@platy
Copy link
Author

platy commented Aug 19, 2016

No it's about the trying to handle anything given and not failing.

The problem that I hit was an array of array of buffer - it is fine the Buffer contains a utf8 string (or something which is a valid string) as the elements in the array are converted to strings and appended, the effect is just to remove the extra array.

> [[new Buffer('foo')]].map(String)
[ 'foo' ] 
> [[Buffer.of(0, 1, 2)]].map(String)
[ '\u0000\u0001\u0002' ]
> [[Buffer.of(0, 1, 2)]].map(String).map(Buffer)
[ <Buffer 00 01 02> ]

In my system this was the case for all of the tests and in production hit problems.

> [[Buffer.of(129, 130)]].map(String).map(Buffer)
[ <Buffer ef bf bd ef bf bd> ]

If we only accept strings and buffers, and don't make silent conversions, these problems are much easier to detect in tests.

@ronkorving
Copy link
Collaborator

Fair enough, I don't mind it being more strict about it. Strings and buffers (and arrays of them) only then 👍

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

No branches or pull requests

2 participants