It looks like we've agreed on everything except for the status codes and the
spec for what WebSocket does after a CS returned from onClose completes.
May I propose the following spec?
/**
* Receives a Close message.
*
...
*
* <p> A Close message consists of a status code and a reason for
* closing. The status code is an integer in the range {@code 1000 <=
* code <= 65535}. The {@code reason} is a short string that has an
* UTF-8 representation not longer than {@code 123} bytes. For more
* details on Close message, status codes and reason see RFC 6455 sections
* <a href="https://tools.ietf.org/html/rfc6455#section-5.5.1";>5.5.1.
Close</a>
* and
* <a href="https://tools.ietf.org/html/rfc6455#section-7.4";>7.4. Status
Codes</a>.
*
* <p> After the returned {@code CompletionStage} has completed
* (normally or exceptionally), the {@code WebSocket} finishes the
* Closing handshake by sending an appropriate Close message. This
* implementation sends a Close message with the same code this one has
* and an empty reason.
*
...
*
*/
CompletionStage<?> onClose(WebSocket webSocket, int statusCode, String
reason)
and
/**
* Sends a Close message with the given status code and the reason.
*
...
*
* <p> The {@code statusCode} is an integer in the range {@code 1000 <= code
* <= 4999}. However, not all status codes may be legal in some
* implementations. {@code 1000} (Normal Closure) is always legal and {@code
* 1002}, {@code 1003}, {@code 1005}, {@code 1006}, {@code 1007}, {@code
* 1009}, {@code 1010}, {@code 1012}, {@code 1013} and {@code 1015} are are
* always illegal codes.
*
* <p> The {@code reason} is a short string that must have an UTF-8
* representation not longer than {@code 123} bytes. For more details on
* Close message, status codes and reason see RFC 6455 sections
* <a href="https://tools.ietf.org/html/rfc6455#section-5.5.1";>5.5.1.
Close</a>
* and
* <a href="https://tools.ietf.org/html/rfc6455#section-7.4";>7.4. Status
Codes</a>.
*
...
*
* @throws IllegalArgumentException
* if the {@code statusCode} has an illegal value, or
* {@code reason} doesn't have an UTF-8 representation not longer
* than {@code 123} bytes
*
* @see #sendClose()
*/
CompletableFuture<WebSocket> sendClose(int statusCode, String reason);
Oh dear, I know Simone won't probably like it, but please bear with me while I'm
trying to explain the rationale behind all this.
1. onClose now clearly defines what happens when returned CS completes. And the
default implementation follows the RFC's suggestion:
When sending a Close frame in response, the endpoint typically echos the
status code it received.
2. The asymmetry between range of codes [1000, 65535] eligible for onClose and
range of codes [1000, 4999] eligible for sendClose is due applying Robustness
Principle [1] to spec's vagueness [2], [3] in this area.
In other words, it's playing safe to accept codes from [5000, 65535], but at the
same time not allowing to send them.
3. Allowing/rejecting particular codes is hard. Having carefully reviewed all
defined codes I would propose to go with this:
1000 is always allowed (along with the no-args sendClose() that sends an
empty Close message)
1002, 1003, 1005, 1006, 1007, 1009, 1010, 1012, 1013 and 1015 are always
rejected
Everything else is up to an implementation. The reason is that otherwise we
could spend too much time discussing these codes, simply because except for the
ones above their description is too vague and differ in RFC and IANA. So if
anybody wants to rathole, let's fork a separate thread for it.
Here's a small notation that might help with reasoning about status codes:
S: @implSpec N: @implNote
+: allow -: reject
Here we go:
S+ 1000 Normal Closure
N+ 1001 Going Away
What's the difference between this and 1000? It seems to be way too
subtle understand from a short description in the javadoc.
As far as I understand it's designed a premature close (cancellation).
Maybe it's okay to send it in the midst of receiving something?
S- 1002 Protocol error
Should not be allowed as the only one who decides what the protocol
error is, is the WebSocket implementation.
S- 1003 Unsupported Data
Ideally should be automatically sent by WebSocket in case the
corresponding method is not overridden and a message of this type has
been received.
It could've been done by having a separate handlers for all types of
events instead of a single Listener type... but we've chosen a different
way. Maybe we could configure this behaviour later in Builder.
N- 1004 Reserved
From the RFC:
"Reserved. The specific meaning might be defined in the future."
I think it's ok to reject it for now, but let's not add this to the
spec just yet.
S- 1005 No Status Rcvd
It's not going to be accepted and translated to an empty Close message
by the impl. Because it's way too deceptive and non-intuitive.
S- 1006 Abnormal Closure
S- 1007 Invalid frame payload data
As with 1002, this should be allowed to be set only by WebSocket.
N+ 1008 Policy Violation
This one is not that clear. Supertype for 1003/1009. What is a policy?
S- 1009 Message Too Big
As with 1003 and ideal candidate to be automatically set by WebSocket,
given that maximum allowed size is configurable in Builder.
It can be done manually today, but with extensions user might not be
able to receive a fragment, and the message will have to be fully
assembled before it relayed to Listener.
S- 1010 Mandatory Ext.
Same as with 1002, 1007, 1003, etc. Should be only set by WebSocket.
N+ 1011 Internal Error
IANA describes it as Internal Error, however RFC's initial text is this:
"1011 indicates that a server is terminating the connection because
it encountered an unexpected condition that prevented it from
fulfilling the request."
So does it have to be linked with some request? Or it can also mean an
asynchronous error, not caused by any of the previous requests?
S- 1012 Service Restart
S- 1013 Try Again Later
These two are new ones. They have been added to IANA registry after the
RFC has been published.
It looks like they are more about the server [4].
N- 1014 Unassigned
Same as with 1004. Reject, but don't mention in the soec.
S- 1015 TLS handshake
N- 1016-3999 Unassigned
This ones should not be allowed for the same reason 1004 and 1014 are
not allowed. Reject them, but not mention in the spec.
N+ 4000-4999 Reserved for Private Use
Use it if you know what they mean to a particular server.
Phew!
Thanks,
-Pavel
--------------------------------------------------------------------------------
[1] https://en.wikipedia.org/wiki/Robustness_principle
[2] https://tools.ietf.org/html/rfc6455#section-7.4.2
[3]
http://autobahn.ws/testsuite/reports_20131013/clients/index.html#case_desc_7_13_1
[4] http://www.ietf.org/mail-archive/web/hybi/current/msg09670.html
> On 17 Jun 2016, at 16:15, Pavel Rappo <pavel.ra...@oracle.com> wrote:
>
> Hi,
>
> Could you please [*] review the following change for the upcoming JDK-8159053?
> This change addresses:
>
> 1. Listener.onClose/onPing behaviour, making the implementation fully*
> responsible of protocol compliance. So reactions on onClose/onPing cannot be
> overridden/redefined in a Listener
>
> 2. Simpler representation of the Close message.
>
> /**
> * Receives a Pong message.
> *
> * <p> A Pong message may be unsolicited or may be received in response
> * to a previously sent Ping. In the latter case, the contents of the
> * Pong is identical to the originating Ping.
> *
> * <p> The message will consist of not more than {@code 125} bytes:
> * {@code message.remaining() <= 125}.
> *
> * <p> The {@code onPong} method is invoked zero or more times in
> * between {@code onOpen} and ({@code onClose} or {@code onError}).
> *
> * <p> If an exception is thrown from this method or the returned {@code
> * CompletionStage} completes exceptionally, then {@link
> * #onError(WebSocket, Throwable) onError} will be invoked with this
> * exception.
> *
> * @implSpec The default implementation behaves as if:
> *
> * <pre>{@code
> * webSocket.request(1);
> * return CompletableFuture.completedStage(null);
> * }</pre>
> *
> * @param webSocket
> * the WebSocket
> * @param message
> * the message
> *
> * @return a CompletionStage that completes when the message processing
> * is done; or {@code null} if already done
> */
> default CompletionStage<?> onPong(WebSocket webSocket,
> ByteBuffer message) {
> webSocket.request(1);
> return null;
> }
>
> /**
> * Receives a Close message.
> *
> * <p> Once a Close message is received, the server will not send any
> * more messages.
> *
> * <p> A Close message consists of a status code and a reason for
> * closing. The status code is an integer in the range {@code 1000 <=
> * code <= 65535}. The reason is a short string that has an UTF-8
> * representation not longer than {@code 123} bytes.
> *
> * <p> For more information on status codes and reasons see
> * <a href="https://tools.ietf.org/html/rfc6455#section-7.4";>RFC 6455,
> 7.4. Status Codes</a>.
> *
> * <p> {@code onClose} is the last invocation on the {@code Listener}.
> * It is invoked at most once, but after {@code onOpen}. If an exception
> * is thrown from this method, it is ignored.
> *
> * <p> After a Close message has been received, the implementation will
> * close the connection automatically. However, the client is allowed to
> * finish sending the current sequence of partial message first. To
> * facilitate it, the WebSocket implementation expects this method to
> * return a {@code CompletionStage}. The implementation then will
> * attempt to close the connection at the earliest of, either the
> * completion of the returned {@code CompletionStage} or the last part
> * of the current message has been sent.
> *
> * @implSpec The default implementation behaves as if:
> *
> * <pre>{@code
> * return CompletableFuture.completedStage(null);
> * }</pre>
> *
> * @param webSocket
> * the WebSocket
> * @param statusCode
> * the status code
> * @param reason
> * the reason
> *
> * @return a CompletionStage that when completed will trigger closing of
> * the WebSocket
> */
> default CompletionStage<?> onClose(WebSocket webSocket,
> int statusCode,
> String reason) {
> return null;
> }
>
> /**
> * Sends a Close message with the given status code and the reason.
> *
> * <p> Returns a {@code CompletableFuture<WebSocket>} which completes
> * normally when the message has been sent or completes exceptionally if an
> * error occurs.
> *
> * <p> The {@code statusCode} is an integer in the range {@code 1000 <=
> code
> * <= 4999}, except for {@code 1004}, {@code 1005}, {@code 1006} and {@code
> * 1015}. The {@code reason} is a short string that must have an UTF-8
> * representation not longer than {@code 123} bytes.
> *
> * <p> For more information about status codes see
> * <a href="https://tools.ietf.org/html/rfc6455#section-7.4";>RFC 6455,
> 7.4. Status Codes</a>.)
> *
> * <p> If a Close message has been already sent or the {@code WebSocket} is
> * closed, then invoking this method has no effect and returned {@code
> * CompletableFuture} completes normally.
> *
> * <p> The returned {@code CompletableFuture} can complete exceptionally
> * with:
> * <ul>
> * <li> {@link IOException}
> * if an I/O error occurs during this operation
> * </ul>
> *
> * @param statusCode
> * the status code
> * @param reason
> * the reason
> *
> * @return a CompletableFuture with this WebSocket
> *
> * @throws IllegalArgumentException
> * if the {@code statusCode} has an illegal value, or
> * {@code reason} doesn't have an UTF-8 representation not longer
> * than {@code 123} bytes
> *
> * @see #sendClose()
> */
> CompletableFuture<WebSocket> sendClose(int statusCode, String reason);
>
> /**
> * Sends an empty Close message.
> *
> * <p> Returns a {@code CompletableFuture<WebSocket>} which completes
> * normally when the message has been sent or completes exceptionally if an
> * error occurs.
> *
> * <p> If a Close message has been already sent or the {@code WebSocket} is
> * closed, then invoking this method has no effect and returned {@code
> * CompletableFuture} completes normally.
> *
> * <p> The returned {@code CompletableFuture} can complete exceptionally
> * with:
> * <ul>
> * <li> {@link IOException}
> * if an I/O error occurs during this operation
> * </ul>
> *
> * @return a CompletableFuture with this WebSocket
> */
> CompletableFuture<WebSocket> sendClose();
>
> Thanks,
> -Pavel
>
> --------------------------------------------------------------------------------
> [1] Please excuse me for not publishing this in a form of a webrev. The reason
> is that currently there are too many outstanding changes in the queue that
> would
> simply obscure the review.
