Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Figure out how we are going to deal with potentially broken UTF8 #106

Closed
3rd-Eden opened this Issue · 8 comments

4 participants

@3rd-Eden

If we start sending over UTF8, the connection starts breaking down (if you are using websockets)

(Note, this uses the new streaming api that I'm working on)

'use strict';

// setup engine.io
var http = require('http').createServer()
  , engine = require('./index').attach(http)
  , fs = require('fs');

// engine.io client
var client = require('engine.io-client');

engine.on('connection', function (socket) {
  console.log('connection', __filename);
  var stream = fs.createReadStream('/dev/random');

  // Kill the stream, socket closed biatches
  socket.on('close', function () {
    stream.destroy();
  });

  stream.pipe(socket);
});

var socket = new client.Socket('ws://localhost:8080');

socket.onopen = function () {
  console.log('socket open', 'socket has been opened');
};

socket.onmessage = function (data) {
  console.log('received', data.data.length);
};

socket.onclose = function () {
  console.log('closed the socket');
};

// Output info
http.listen(8080);

The fix for this would be doing a JSON.stringify on the data before we send it over the WebSocket connection.

@rauchg
Owner

We should also check out what ServerResponse.write is doing with the buffer before it makes it to the net.Stream.

@rauchg
Owner

Actually we should look at what's making ws error out instead.

@mjgil

Hey guys, I just ran this with my own txt file and was wondering if this was the error to be fixed.

stream.js:66
dest.end();
^
TypeError: Object # has no method 'end'
at onend (stream.js:66:10)
at EventEmitter.emit (events.js:126:20)
at afterRead (fs.js:1330:12)
at Object.wrapper as oncomplete

@rauchg
Owner

@mjgil no, he's using an experimental stream branch he was working on. You should instead .send directly instead of using .pipe

@mjgil mjgil referenced this issue in Automattic/engine.io-protocol
Closed

adding first utf8 encoding fix #11

@rauchg
Owner

@3rd-Eden do you think that pull is a good fix ^ ?

@3rd-Eden

@guille it looks legit, but haven't tested it.

@defunctzombie
Collaborator

Was this fixed? Has the issue gone away?

@defunctzombie
Collaborator

Based on some recent commits I think this is resolved and should be closed. If there are issues discovered they should be new issues with failing cases.

@rauchg rauchg closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.