WebSocket client API

[Peter Levart]

On 10/20/2015 12:49 PM, Simone Bordet wrote:
> Hi,
>
> On Tue, Oct 20, 2015 at 11:07 AM, Peter Levart <peter.levart at gmail.com> wrote:
>> Hi,
>>
>> Another thought about [Char|Byte]Buffer recycling. If the onXXX call-backs
>> returned a CF so that they could asynchronously signal when they are done
>> with consumption, the signature of the method could be:
>>
>> CompletionStage<CharBuffer> onText(..., CharBuffer cb, ...)
>>
>> Implementor of the method (the user) could do the following things:
>>
>> - return null or CompletableFuture.completedStage(null) to signal that the
>> method already performed the consumption synchronously, but that it retained
>> the CharBuffer, so WebSocket should not recycle it, but must consider it
>> lost.
>> - return CompletableFuture.completedStage(cb) to signal that the method
>> already performed the consumption synchronously and the result of completed
>> CompletionStage is the buffer that can be returned into the internal pool of
>> WebSocket buffers.
>> - return a CompletionStage that is yet to be completed asynchronously with
>> the 'cb' or null as the result. The first case returns the buffer to the
>> pool, the 2nd signals to WebSocket that the buffer is lost.
>>
>> This way, buffer recycling is in the domain of WebSocket implementation -
>> not the user of WebSocket.
>>
>> The return type of onXXX methods should be CompletionStage not
>> CompletableFuture. CompletionStage is not a Future and does not have methods
>> that allow canceling of the underlying computation. All that WebSocket needs
>> is attaching a completion that recycles the buffer, like:
>>
>> CharBuffer message = ... get buffer from pool or create new one ...
>>
>> ... fill message with data ...
>>
>> CompletionStage<CharBuffer> cs = listener.onText(..., message, ...);
>>
>> if (cs != null) {
>>      cs.thenAccept(cb -> {
>>          if (cb != null) {
>>              .... return cb to buffer pool ...
>>          }
>>      });
>> }
>>
>> What do you think?
> The ability to return null to be completely equivalent to returning
> CompletableFuture.completedFuture(cb) would cover a common case
> (synchronous consumption) without incurring in forced allocation.
> I am not sure how common is the case to synchronously consume the
> buffer, but then not returning it.
> Perhaps your cases could be reduced to:
>
> * return null or CompletableFuture.completedFuture(cb) to indicate
> full recycle of the buffer.
> * return CompletableFuture.completedFuture(null) to indicate that the
> buffer is lost.
>
> This would reduce the allocation cost to just the fully async consumption case.
>
> With your proposal the typical async proxy case would then write from:
>
> onText(WebSocket ws, CharBuffer cb, boolean isLast)
> {
>      return ws.sendText(cb, isLast).thenAccept(_ -> ws.request(1));
> }
>
> to:
>
> onText(WebSocket ws, CharBuffer cb, boolean isLast)
> {
>      return ws.sendText(cb, isLast).thenApply(_ -> { ws.request(1); return cb });
> }
>
> Just slightly more complex.
>
> However, would it not be a potential attack vector ?
> Applications would be able to "inject" a buffer into the
> implementation buffer pool, and be able to peek/modify bytes from
> other connections.
> It should be:
>
> if (cs != null) {
>      cs.thenAccept(cb -> {
>          if (cb != null) {
>              .... return *message* to buffer pool ...
>          }
>      });
> }
>
> At that point, the return value of onText() could well be a CF<?> like
> it already is, no ?
> The implementation would just need to test for null, but then never
> actually use the returned object wrapped by the CF.
>
> What do you think ?
>

You're right about potential attack vector.

Then perhaps a CompletionStage<Boolean> is more appropriate return 
signature. But not more safe.

Implementation could lie and return non-null (or true) indicating that 
buffer can be re-used, but in fact retaining it and peeking at other 
connection's data...

To be safe, WebSocket would have to maintain separate pool of buffers 
for each connection. The pool could be shared among connections if it 
was given to WebSocket.Builder by the user. This could be implicit if 
API was designed with another level of indirection:

interface WebSocket {

     static Factory newFactory() {
         // this could also be used to look-up the implementation with 
ServiceLoader
     }

     abstract class Factory {
         public abstract Builder newBuilder();
     }

     abstract class Builder {
         ...
     }
}


The buffer pool would be on the Factory level. All sockets built by 
Builders constructed from the same Factory instance would share the same 
pool.

Regards, Peter