Socket write encoding case sensitivity #1586

Closed
wants to merge 1 commit into from

3 participants

@ajclarkedubit

Just spent ages debugging a problem with Node failing an assertion when sending messages along a socket - it turns out that I was specifying "UTF8" as the encoding value, which was not being recognised because it's uppercase.

So I've just added a line to lowercase the encoding value and maybe save someone else some pain!

@bnoordhuis
Node.js Foundation member

I don't know. Socket.write() is hot code, it should be as fat free as possible. I'd rather solve this by making the docs more explicit.

@koichik?

@koichik

@bnoordhuis

I'd rather solve this by making the docs more explicit.

Because most of functions such as a Buffer() accept encoding name by uppercase, I think that we should fix this problem.

But, it is not only 'utf8' but also 'ucs2', 'hex' and 'base64' that the number of characters and the number of bytes are different.
Therefore:

  • Buffer.write() should always set _charsWritten.
  • Socket._writeOut() should not care its encoding.

Or, as well as net_uv.js:

  • Socket._writeOut() should create new Buffer rather than reusing it.
@koichik koichik added a commit to koichik/node that referenced this pull request Sep 3, 2011
@koichik koichik net: Socket write encoding case sensitivity
Fixes #1586.
55e6be1
@koichik

Buffer.write() always sets Buffer._charsWritten by #1633, Socket._writeOut() should not care its encoding.
@bnoordhuis - Can you review 55e6be1?

@koichik koichik added a commit that closed this pull request Sep 4, 2011
@koichik koichik net: Socket write encoding case sensitivity
Fixes #1586.
fdbfc9c
@koichik koichik closed this in fdbfc9c Sep 4, 2011
@koichik

@ajclarke - This has just fixed in the another way. Thanks for the report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment