engine.io client marks transport as open too early

[das7pad]

You want to:

• report a bug
• request a feature

Current behaviour

Client can send unauthenticated messages on (failing?) connection startup.

Steps to reproduce (if the current behaviour is a bug)

Connect via socket.io client from Firefox.
We see a failure rate of about 1 in 2000 attempts.

engine.io server is configured with cookie: false -- use query parameter for
auth.

Expected behaviour

Always wait for successful handshake.

Setup

• OS: Windows/Mac/Linux
• Browser: Firefox (old 6x -> latest 76)
• engine.io-client version: 3.4.0 (bundled via socket.io-client 2.3.0)
• engine.io version: 3.4.1

Other information (e.g. stacktraces, related issues, suggestions how to fix)

Hi!

We are currently migrating to socket.io v2 (engine.io v3) and are experiencing
some issues. The below issue is about a failing handshake (polling).

The handshake is on the engine.io layer, but if you think it is higher in the
stack, please move it.

I found a socket.io POST requests in the nginx logs that had no
sid (socket id) query parameter set -- aka are not authenticated.

Currently a connection startup roughly works like this:

• client: init new socket, mark as not writable
• client: start polling (GET request, unauthenticated)
• server: sends handshake blob including sid
• client: mark socket as writable
• client: process the handshake blob
• client: put sid in the URL query object for following requests

Upon inspection of the code paths server/client side I am confident that this
is either caused by a (silent) server error which was not handled properly by
the client, a client side race condition or a parse error.

Currently any first packet marks the socket as open, this includes error
messages. Given that only the successful handshake contains the sid, we can end
up with a socket that is marked as ready for writes (allows POST requests), but
has no authentication added to the query.

engine.io-client/lib/transports/polling.js

Lines 132 to 135 in 7aad0d6

	// if its the first message we consider the transport open
	if ('opening' === self.readyState) {
	self.onOpen();
	}

The race condition: client: mark socket as writable has to run AFTER sid
propagated -- otherwise the socket could accept/process write requests that
result in unauthenticated requests.

A log line with the 400 response code from the unauthenticated POST request
has a response size of 43, which we can translate to the error code 1 or
'Session ID unknown' [1]

"POST /socket.io.v2/?EIO=3&transport=polling&t=N9MW1yd HTTP/2.0" 400 43

Prior log lines of the initial polling/open request show a payload size of 104,
which is equal to other successful handshakes.

"GET /socket.io.v2/?EIO=3&transport=polling&t=N9MW1t4 HTTP/2.0" 200 104

> '97:0{"sid":"O3fVn-i4EaMkO-NXAAAU","upgrades":["websocket"],"pingInterval":25000,"pingTimeout":60000}2:40'.length
104

Given that the initial payload size matches the expected payload size, I do not
think that the payload was an error payload. Which suggests either the race
condition -- or a parse error.

This is happening for Firefox users only.

---

[1] error message -> payload size mapping:

https://github.com/socketio/engine.io/blob/001ca62cc4a8f511f3b2fbd9e4493ad274a6a0e5/lib/server.js#L266-L270
https://github.com/socketio/engine.io/blob/001ca62cc4a8f511f3b2fbd9e4493ad274a6a0e5/lib/server.js#L80-L86

> Object.entries({0: 'Transport unknown', 1: 'Session ID unknown', 2: 'Bad handshake method', 3: 'Bad request', 4: 'Forbidden'}).forEach(([code, message])=> console.log({length: JSON.stringify({code, message}).length, code, message}))
{ length: 42, code: '0', message: 'Transport unknown' }
{ length: 43, code: '1', message: 'Session ID unknown' }
{ length: 45, code: '2', message: 'Bad handshake method' }
{ length: 36, code: '3', message: 'Bad request' }
{ length: 34, code: '4', message: 'Forbidden' }

[darrachequesne]

The following

"POST /socket.io.v2/?EIO=3&transport=polling&t=N9MW1yd HTTP/2.0" 400 43

should indeed never happen (a POST without sid). That's weird. Could this be linked to using HTTP/2?

[das7pad]

Hi, thank you for taking a look at this issue!

We have seen unauthenticated POST requests without an sid and using http/1.1 as well. But the response payload is different and the error rate is much lower -- below 1% of the 400 43 rate.

"POST /socket.io.v2/?EIO=3&transport=polling&t=STAMP HTTP/1.1" 400 54

STAMP being a placeholder.

Pipelining via HTTP/2 helps in getting the unauthenticated request out, but it should have never come to the point of emitting an unauthenticated request.

[darrachequesne]

Fixed:

• in the master branch: 1c8cba8 (included in engine.io-client@4.0.4)
• backported to the 3.5.x branch: 8750356 (included in engine.io-client@3.5.0)

Thanks a lot for the detailed report 🏆

[das7pad]

Great, thank you for fixing this bug.