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

Support 1012, 1013 and 1014 WebSocket close status code #8664

Merged
merged 1 commit into from Dec 17, 2018

Conversation

Projects
None yet
3 participants
@slandelle
Copy link
Contributor

slandelle commented Dec 14, 2018

Motivation:

RFC 6455 doesn't define close status codes 1012, 1013 and 1014.
Yet, since then, IANA has defined them and web browsers support them.

From https://www.iana.org/assignments/websocket/websocket.xhtml:

  • 1012: Service Restart
  • 1013: Try Again Later
  • 1014: The server was acting as a gateway or proxy and received an invalid response from the upstream server. This is similar to 502 HTTP Status Code.

Modification:

Make status codes 1012, 1013 and 1014 legit.

Result:

WebSocket status codes as defined by IANA are supported.

@slandelle

This comment has been minimized.

Copy link
Contributor Author

slandelle commented Dec 14, 2018

Investigating if autobahn test suite can support those new codes.

[ERROR] Failed to execute goal me.normanmaurer.maven.autobahntestsuite:autobahntestsuite-maven-plugin:0.1.4:fuzzingclient (default) on project netty-testsuite-autobahn: 
[ERROR] Failed test cases:
[ERROR] 	[7.9.8] behavior: FAILED, behaviorClose: WRONG_CODE, duration: 2ms, remoteCloseCode: 1014, reportFile: target/autobahntestsuite-reports/autobahntestsuite_maven_plugin_case_7_9_8.json
[ERROR] 	[7.9.6] behavior: FAILED, behaviorClose: WRONG_CODE, duration: 2ms, remoteCloseCode: 1012, reportFile: target/autobahntestsuite-reports/autobahntestsuite_maven_plugin_case_7_9_6.json
[ERROR] 	[7.9.7] behavior: FAILED, behaviorClose: WRONG_CODE, duration: 2ms, remoteCloseCode: 1013, reportFile: target/autobahntestsuite-reports/autobahntestsuite_maven_plugin_case_7_9_7.json
@netty-bot

This comment has been minimized.

Copy link

netty-bot commented Dec 14, 2018

Can one of the admins verify this patch?

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Dec 17, 2018

@slandelle seems like it is "fixed" in recent releases of Autobahntestsuite:

crossbario/autobahn-testsuite#88
crossbario/autobahn-testsuite#52

Let me release an updated version of the maven autobahn testsuite plugin.

@slandelle

This comment has been minimized.

Copy link
Contributor Author

slandelle commented Dec 17, 2018

Great!
Please note that I did’t add 1015 as a valid code.
One the one side, I probably should to stick to the spec.
On the other side, I don’t get how one could get a websocket frame in case of a TLS handshake error. Sadly, the links on the IANA page to the related mailing list are dead.
WDYT?

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Dec 17, 2018

@slandelle yeah... I think what you did is good enough for now...

Support 1012, 1013 and 1014 WebSocket close status code
Motivation:

RFC 6455 doesn't define close status codes 1012, 1013 and 1014.
Yet, since then, IANA has defined them and web browsers support them.

From https://www.iana.org/assignments/websocket/websocket.xhtml:

* 1012: Service Restart
* 1013: Try Again Later
* 1014: The server was acting as a gateway or proxy and received an invalid response from the upstream server. This is similar to 502 HTTP Status Code.

Modification:

Make status codes 1012, 1013 and 1014 legit.

Result:

WebSocket status codes as defined by IANA are supported.

@slandelle slandelle force-pushed the additional-ws-close-status branch from d4e9a18 to 1f06819 Dec 17, 2018

@slandelle

This comment has been minimized.

Copy link
Contributor Author

slandelle commented Dec 17, 2018

@normanmaurer What's the command for triggering CI, please?

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Dec 17, 2018

@netty-bot test this please

@slandelle

@slandelle

This comment has been minimized.

Copy link
Contributor Author

slandelle commented Dec 17, 2018

Thanks!

@normanmaurer normanmaurer self-requested a review Dec 17, 2018

@normanmaurer normanmaurer merged commit 302dac8 into 4.1 Dec 17, 2018

4 checks passed

pull request validation (centos6-java11) Build finished.
Details
pull request validation (centos6-java12) Build finished.
Details
pull request validation (centos6-java8) Build finished.
Details
pull request validation (centos6-java9) Build finished.
Details

@normanmaurer normanmaurer deleted the additional-ws-close-status branch Dec 17, 2018

normanmaurer added a commit that referenced this pull request Dec 17, 2018

Support 1012, 1013 and 1014 WebSocket close status code (#8664)
Motivation:

RFC 6455 doesn't define close status codes 1012, 1013 and 1014.
Yet, since then, IANA has defined them and web browsers support them.

From https://www.iana.org/assignments/websocket/websocket.xhtml:

* 1012: Service Restart
* 1013: Try Again Later
* 1014: The server was acting as a gateway or proxy and received an invalid response from the upstream server. This is similar to 502 HTTP Status Code.

Modification:

Make status codes 1012, 1013 and 1014 legit.

Result:

WebSocket status codes as defined by IANA are supported.
@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Dec 17, 2018

@slandelle thanks... merged!

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