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

Fix for Issue #331 #943

Merged
merged 23 commits into from
Apr 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
75467cd
Issue #331, attach query string to CONNECT packets for secondary name…
zweihan Jan 28, 2016
35ad21f
Merge pull request #2 from whattokingu/queryStringFix
zweihan Jan 30, 2016
fcb176b
Merge commit '5fe7373610ef92de0d34b44d503c22cab5712c8d' into queryStr…
zweihan Jan 31, 2016
017d04c
Merge pull request #5 from whattokingu/queryStringFix
zweihan Jan 31, 2016
74aff26
rebuild client.
zweihan Jan 31, 2016
6d3e85f
Merge pull request #6 from whattokingu/queryStringFix
zweihan Jan 31, 2016
8d44b7a
pass query as Socket property rather than as function params to simpl…
zweihan Jan 31, 2016
5263299
Merge pull request #7 from whattokingu/queryStringFix
zweihan Jan 31, 2016
08e44f3
fix bug and passes tests
zweihan Jan 31, 2016
b878cdb
Revert "rebuild client."
zweihan Jan 31, 2016
3290bb4
move encoding of query string to lookup function.
zweihan Jan 31, 2016
9fa6b1c
update tests
zweihan Jan 31, 2016
34f79d3
Merge pull request #8 from whattokingu/queryStringFix
zweihan Jan 31, 2016
1657971
remove console statement.
zweihan Jan 31, 2016
6564a0c
Merge pull request #9 from whattokingu/queryStringFix
zweihan Jan 31, 2016
3407f3f
update test
paradite Feb 1, 2016
05250b0
code modification to conform to style guide.
zweihan Feb 13, 2016
e3c0f6a
encodes queries using `encodeURIComponent`
zweihan Feb 13, 2016
e0580ef
Merge pull request #10 from whattokingu/queryStringFix
zweihan Feb 13, 2016
bfa4589
fix error with test.
zweihan Feb 13, 2016
f01a349
add back changes made in previous commit
zweihan Feb 13, 2016
4578f7e
Merge branch 'orgmaster' into queryStringFix
zweihan Apr 12, 2016
4fff66d
Merge pull request #12 from whattokingu/queryStringFix
zweihan Apr 12, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,27 @@ function lookup (uri, opts) {
}
io = cache[id];
}

return io.socket(parsed.path);
if (parsed.query && !opts.query) {
opts.query = parsed.query;
} else if (opts && 'object' === typeof opts.query) {
opts.query = encodeQueryString(opts.query);
}
return io.socket(parsed.path, opts);
}
/**
* Helper method to parse query objects to string.
* @param {object} query
* @returns {string}
*/
function encodeQueryString (obj) {
var str = [];
for (var p in obj) {
if (obj.hasOwnProperty(p)) {
str.push(encodeURIComponent(p) + '=' + encodeURIComponent(obj[p]));
}
}
return str.join('&');
}

/**
* Protocol version.
*
Expand Down
7 changes: 4 additions & 3 deletions lib/manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ Manager.prototype.maybeReconnectOnOpen = function () {
*/

Manager.prototype.open =
Manager.prototype.connect = function (fn) {
Manager.prototype.connect = function (fn, opts) {
debug('readyState %s', this.readyState);
if (~this.readyState.indexOf('open')) return this;

Expand Down Expand Up @@ -350,10 +350,10 @@ Manager.prototype.onerror = function (err) {
* @api public
*/

Manager.prototype.socket = function (nsp) {
Manager.prototype.socket = function (nsp, opts) {
var socket = this.nsps[nsp];
if (!socket) {
socket = new Socket(this, nsp);
socket = new Socket(this, nsp, opts);
this.nsps[nsp] = socket;
var self = this;
socket.on('connecting', onConnecting);
Expand Down Expand Up @@ -400,6 +400,7 @@ Manager.prototype.destroy = function (socket) {
Manager.prototype.packet = function (packet) {
debug('writing packet %j', packet);
var self = this;
if (packet.query && packet.type === 0) packet.nsp += '?' + packet.query;

if (!self.encoding) {
// encode, then write to engine with result
Expand Down
11 changes: 9 additions & 2 deletions lib/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ var emit = Emitter.prototype.emit;
* @api public
*/

function Socket (io, nsp) {
function Socket (io, nsp, opts) {
this.io = io;
this.nsp = nsp;
this.json = this; // compat
Expand All @@ -62,6 +62,9 @@ function Socket (io, nsp) {
this.sendBuffer = [];
this.connected = false;
this.disconnected = true;
if (opts && opts.query) {
this.query = opts.query;
}
if (this.io.autoConnect) this.open();
}

Expand Down Expand Up @@ -183,7 +186,11 @@ Socket.prototype.onopen = function () {

// write connect packet if necessary
if ('/' !== this.nsp) {
this.packet({ type: parser.CONNECT });
if (this.query) {
this.packet({type: parser.CONNECT, query: this.query});
} else {
this.packet({type: parser.CONNECT});
}
}
};

Expand Down
13 changes: 13 additions & 0 deletions test/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,17 @@ describe('socket', function () {
socket.compress(false).emit('hi');
});
});

it('should store query string as a property', function (done) {
var socket = io('/abc', {query: {a: 'b'}}); // passes in as a query obj
var socket2 = io('/abcd?b=c&d=e'); // passes in as a query string
var socket3 = io('/abc', {query: {'&a': '&=?a'}}); // checks that it encodes a string
expect(socket.query).to.be('a=b');
expect(socket2.query).to.be('b=c&d=e');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if we could test if query parameter was actually sent to server?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The corresponding PR on socket.io has the test that checks for the query parameter being sent over. Do you think that's ok?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it would be ok, thanks!

expect(socket3.query).to.be('%26a=%26%3D%3Fa');
socket.disconnect();
socket2.disconnect();
socket3.disconnect();
done();
});
});