Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

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
Owner

@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 referenced this pull request from a commit in koichik/node
@koichik koichik net: Socket write encoding case sensitivity
Fixes #1586.
55e6be1
@koichik
Owner

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

@koichik koichik closed this in fdbfc9c
@koichik
Owner

@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
This page is out of date. Refresh to see the latest.
Showing with 5 additions and 0 deletions.
  1. +5 −0 lib/net_legacy.js
View
5 lib/net_legacy.js
@@ -411,6 +411,11 @@ Socket.prototype._writeOut = function(data, encoding, fd, cb) {
pool = null;
allocNewPool();
}
+
+ if (encoding)
+ {
+ encoding = String(encoding).toLowerCase();
+ }
if (!encoding || encoding == 'utf8' || encoding == 'utf-8') {
// default to utf8
Something went wrong with that request. Please try again.