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

NullPointerException in receiveHandshakeResponse() because HTTP_HEADER_UPGRADE is not in the response #234

Closed
1 of 2 tasks
miketran78727 opened this issue Aug 10, 2016 · 4 comments
Assignees
Milestone

Comments

@miketran78727
Copy link
Contributor

Please fill out the form below before submitting, thank you!

  • Bug exists Release Version 1.1.0 ( Master Branch)
  • Bug exists in Snapshot Version 1.1.1-SNAPSHOT (Develop Branch)

The broker is MessageSight.

** Copied from email thread paho-dev@eclipse.org
Hi Mike, 

Which broker are you using for this? The HTTP_HEADER_UPGRADE header is required to change the initial http connection into a web socket and so the null pointer is correct (though a better error might be a good improvement for 1.3.0). I've tested this with the iot.eclipse.leg Mosquitto, but not with other brokers so can't guarantee the results on them...

On Wed, 10 Aug 2016, 22:00 Mike Tran, miketran@us.ibm.com wrote:
Hi James,

Thanks for the reply.. I found this sample in your git account: https://github.com/jpwsutton/EclipsePahoMavenExample/blob/master/src/main/java/org/eclipse/paho/App.java

My code is not much different from yours, but keep hitting NullPointerException exception in receiveHandshakeResponse() because HTTP_HEADER_UPGRADEwas not in the response.  Note that JavaScript client works fine on both 1883 and 443 ports.

NullPointerException on a WebSocket connection:

INFO: main: Connecting client d:xxxxxx:xxxxxx:xxxxxx to ws://xxxxxx.messaging.internetofthings.ibmcloud.com:1883
MqttException (0) - java.lang.NullPointerException
        at org.eclipse.paho.client.mqttv3.internal.ExceptionHelper.createMqttException(ExceptionHelper.java:38)
        at org.eclipse.paho.client.mqttv3.internal.ClientComms$ConnectBG.run(ClientComms.java:664)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
        at org.eclipse.paho.client.mqttv3.internal.websocket.WebSocketHandshake.receiveHandshakeResponse(WebSocketHandshake.java:133)
        at org.eclipse.paho.client.mqttv3.internal.websocket.WebSocketHandshake.execute(WebSocketHandshake.java:74)
        at org.eclipse.paho.client.mqttv3.internal.websocket.WebSocketNetworkModule.start(WebSocketNetworkModule.java:78)
        at org.eclipse.paho.client.mqttv3.internal.ClientComms$ConnectBG.run(ClientComms.java:650)

@miketran78727
Copy link
Contributor Author

MessageSight connection log has this error and close the connection:

2016-08-11T03:38:53.798+00:00 CWLNA1112 warning Connection imaserver 5351: The HTTP handshake for connection from 9.65.129.94:60130 to 9.3.177.75:1883 is not valid: ConnectionID=1860 RC=400 Reason="The WebSockets key is not valid" Data="bXF0dC0xNDcwODg2Nzkz".

@miketran78727
Copy link
Contributor Author

miketran78727 commented Aug 11, 2016

@jpwsutton This is a bug in the client. The WebSockets key is not long enough. When I decoded the string bXF0dC0xNDcwODg2Nzkz I got 15-byte value mqtt-1470886793

https://tools.ietf.org/html/rfc6455#section-4.1

The request MUST include a header field with the name
|Sec-WebSocket-Key|. The value of this header field MUST be a
nonce consisting of a randomly selected 16-byte value that has
been base64-encoded (see Section 4 of [RFC4648]). The nonce
MUST be selected randomly for each connection.

Currently, this is how we generate the code:

String key = "mqtt-" + (System.currentTimeMillis()/1000);
String b64Key = Base64.encode(key);
sendHandshakeRequest(b64Key);
receiveHandshakeResponse(b64Key);

I am working on a fix for this

@jpwsutton jpwsutton added the bug label Sep 7, 2016
@jpwsutton jpwsutton added this to the 1.3.0 milestone Sep 7, 2016
jpwsutton added a commit that referenced this issue Sep 7, 2016
* Adding connection checks to WebSocket Tests

Signed-off-by: James Sutton <james.sutton@uk.ibm.com>

* Making WebSocket Handshake Sec-WebSocket-Key 16 bytes (Issue #234)

Signed-off-by: James Sutton <james.sutton@uk.ibm.com>
@jpwsutton
Copy link
Member

As this is a fairly serious bug and we can't add the UUID fix yet until after 1.3.0, I've opted to change the prefix to 'mqtt3-' which means the whole key is now 16 bytes. We can then improve the key generation after 1.3.0. This change was merged in PR #263

@jpwsutton jpwsutton self-assigned this Sep 7, 2016
@miketran78727
Copy link
Contributor Author

@jpwsutton Sounds good to me.

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

No branches or pull requests

2 participants