RFR [9] 8179021: Latest bugfixes to WebSocket/HPACK from the sandbox repo

Pavel Rappo pavel.rappo at oracle.com
Wed May 10 11:03:42 UTC 2017


Daniel, thanks a lot for looking into this! I've addressed most of your suggestions:

    http://cr.openjdk.java.net/~prappo/8179021/webrev.01/

> On 9 May 2017, at 15:49, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> Hi Pavel,
> 
> A few nits:
> 
> jdk/incubator/http/WebSocket.java:
> 
> line 501: should use {@linkplain
> 
> Also (line 46 and 501) the link target #sendClose may cause trouble
> if sendClose is overloaded - so I wonder if it may be better to
> have the  full prototype there:
>   #sendClose(int, String)
> (though I guess it's OK as long as there is only 1 sendClose method).
> 
> jdk/incubator/http/internal/websocket/OutgoingMessage.java
> 
> 75         // maskingKeys, CharsetDecoder and should be here
> 
> what does this means? are some words missing?
> 
> jdk/incubator/http/internal/websocket/Transmitter.java
> 
>  58      * The supplied handler may be invoked in the calling thread, so watch out
>  59      * for stack overflow, if there's a possibility of send being called again
>  60      * from the handler.
> 
> Can we reformulate this again?
> Something like:
> 
>        * The supplied handler may be invoked in the calling thread.
>        * A {@code StackOverflowException} may thus occur if there's a
>        * possibility that this method is called again by the supplied
>        * handler.
> 
>  93                   // or tuned off
> 
> I guess you meant 'turned off'
> 
> jdk/incubator/http/internal/websocket/WebSocketImpl.java
> 
> 196         if (alreadyCompleted) {
> 197             throw new InternalError();
> ...
> 321         if (!added) {
> 322             throw new InternalError();
> 
> A comment similar to the other places where InternalError is thrown
> would be nice.
> 
> best regards,
> 
> -- daniel
> 
> 
> On 09/05/2017 14:52, Pavel Rappo wrote:
>> Hello,
>> 
>> Please review the following change:
>> 
>>  http://cr.openjdk.java.net/~prappo/8179021/webrev.00/
>> 
>> Along with refactoring this change contains a number of critical fixes for
>> WebSocket. Critical fixes are applied to Receiver, WebSocketImpl and
>> OpeningHandshake.
>> 
>> Also this change removes 2 convenience methods and 1 constant from WebSocket
>> interface. Since this interface is a part of incubation feature there shouldn't
>> be any problem. Those members were initially controversial.
>> 
>> Thanks,
>> -Pavel
>> 
> 



More information about the net-dev mailing list