Issue #3428 - add validation checks when adding decoders

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2020-05-21 18:44:46 +10:00
parent 43e3cdc4e3
commit 9475662451
7 changed files with 81 additions and 52 deletions

View File

@ -138,12 +138,26 @@ public class AvailableDecoders implements Iterable<RegisteredDecoder>
throw new InvalidWebSocketException(err);
}
boolean alreadyRegistered = registeredDecoders.stream().anyMatch(registered ->
registered.decoder.equals(decoder) && registered.interfaceType.equals(interfaceClass));
// Validate the decoder to be added against the existing registered decoders.
for (RegisteredDecoder registered : registeredDecoders)
{
if (!registered.primitive && objectType.equals(registered.objectType))
{
// Streaming decoders can only have one decoder per object type.
if (interfaceClass.equals(Decoder.TextStream.class) || interfaceClass.equals(Decoder.BinaryStream.class))
throw new InvalidWebSocketException("Multiple decoders for objectType" + objectType);
// If decoder is already registered for this interfaceType, don't bother adding it again.
if (!alreadyRegistered)
registeredDecoders.add(0, new RegisteredDecoder(decoder, interfaceClass, objectType, config));
// If we have the same objectType, then the interfaceTypes must be the same to form a decoder list.
if (!registered.interfaceType.equals(interfaceClass))
throw new InvalidWebSocketException("Multiple decoders with different interface types for objectType " + objectType);
}
// If this decoder is already registered for this interface type we can skip adding a duplicate.
if (registered.decoder.equals(decoder) && registered.interfaceType.equals(interfaceClass))
return;
}
registeredDecoders.add(0, new RegisteredDecoder(decoder, interfaceClass, objectType, config));
}
public RegisteredDecoder getFirstRegisteredDecoder(Class<?> type)

View File

@ -31,22 +31,18 @@ import org.eclipse.jetty.websocket.util.messages.MessageSink;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public abstract class AbstractDecodedMessageSink<T extends Decoder> implements MessageSink
public abstract class AbstractDecodedMessageSink implements MessageSink
{
protected final Logger _logger;
protected final CoreSession _coreSession;
protected final MethodHandle _methodHandle;
protected final MessageSink _messageSink;
protected final List<T> _decoders;
public AbstractDecodedMessageSink(CoreSession coreSession, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
public AbstractDecodedMessageSink(CoreSession coreSession, MethodHandle methodHandle)
{
_logger = LoggerFactory.getLogger(getClass());
_coreSession = coreSession;
_methodHandle = methodHandle;
_decoders = decoders.stream()
.map(RegisteredDecoder::<T>getInstance)
.collect(Collectors.toList());
try
{
@ -72,4 +68,32 @@ public abstract class AbstractDecodedMessageSink<T extends Decoder> implements M
_logger.debug("accepting frame {} for {}", frame, _messageSink);
_messageSink.accept(frame, callback);
}
public abstract static class Basic<T extends Decoder> extends AbstractDecodedMessageSink
{
protected final List<T> _decoders;
public Basic(CoreSession coreSession, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
{
super(coreSession, methodHandle);
if (decoders.isEmpty())
throw new IllegalArgumentException("Require at least one decoder for " + this.getClass());
_decoders = decoders.stream()
.map(RegisteredDecoder::<T>getInstance)
.collect(Collectors.toList());
}
}
public abstract static class Stream<T extends Decoder> extends AbstractDecodedMessageSink
{
protected final T _decoder;
public Stream(CoreSession coreSession, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
{
super(coreSession, methodHandle);
if (decoders.size() != 1)
throw new IllegalArgumentException("Require exactly one decoder for " + this.getClass());
_decoder = decoders.get(0).getInstance();
}
}
}

View File

@ -33,7 +33,7 @@ import org.eclipse.jetty.websocket.javax.common.decoders.RegisteredDecoder;
import org.eclipse.jetty.websocket.util.messages.ByteBufferMessageSink;
import org.eclipse.jetty.websocket.util.messages.MessageSink;
public class DecodedBinaryMessageSink<T> extends AbstractDecodedMessageSink<Decoder.Binary<T>>
public class DecodedBinaryMessageSink<T> extends AbstractDecodedMessageSink.Basic<Decoder.Binary<T>>
{
public DecodedBinaryMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
{

View File

@ -33,9 +33,9 @@ import org.eclipse.jetty.websocket.javax.common.decoders.RegisteredDecoder;
import org.eclipse.jetty.websocket.util.messages.InputStreamMessageSink;
import org.eclipse.jetty.websocket.util.messages.MessageSink;
public class DecodedBinaryStreamMessageSink<T> extends AbstractDecodedMessageSink<Decoder.BinaryStream<T>>
public class DecodedBinaryStreamMessageSink<T> extends AbstractDecodedMessageSink.Stream<Decoder.BinaryStream<T>>
{
public DecodedBinaryStreamMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders) throws NoSuchMethodException, IllegalAccessException
public DecodedBinaryStreamMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
{
super(session, methodHandle, decoders);
}
@ -52,22 +52,18 @@ public class DecodedBinaryStreamMessageSink<T> extends AbstractDecodedMessageSin
@SuppressWarnings("Duplicates")
public void onStreamStart(InputStream stream)
{
for (Decoder.BinaryStream<T> decoder : _decoders)
try
{
try
{
T obj = decoder.decode(stream);
_methodHandle.invoke(obj);
return;
}
catch (DecodeException e)
{
throw new CloseException(CloseReason.CloseCodes.CANNOT_ACCEPT.getCode(), "Unable to decode", e);
}
catch (Throwable t)
{
throw new CloseException(CloseReason.CloseCodes.CANNOT_ACCEPT.getCode(), "Endpoint notification error", t);
}
T obj = _decoder.decode(stream);
_methodHandle.invoke(obj);
}
catch (DecodeException e)
{
throw new CloseException(CloseReason.CloseCodes.CANNOT_ACCEPT.getCode(), "Unable to decode", e);
}
catch (Throwable t)
{
throw new CloseException(CloseReason.CloseCodes.CANNOT_ACCEPT.getCode(), "Endpoint notification error", t);
}
}
}

View File

@ -32,9 +32,9 @@ import org.eclipse.jetty.websocket.javax.common.decoders.RegisteredDecoder;
import org.eclipse.jetty.websocket.util.messages.MessageSink;
import org.eclipse.jetty.websocket.util.messages.StringMessageSink;
public class DecodedTextMessageSink<T> extends AbstractDecodedMessageSink<Decoder.Text<T>>
public class DecodedTextMessageSink<T> extends AbstractDecodedMessageSink.Basic<Decoder.Text<T>>
{
public DecodedTextMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders) throws NoSuchMethodException, IllegalAccessException
public DecodedTextMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
{
super(session, methodHandle, decoders);
}

View File

@ -33,9 +33,9 @@ import org.eclipse.jetty.websocket.javax.common.decoders.RegisteredDecoder;
import org.eclipse.jetty.websocket.util.messages.MessageSink;
import org.eclipse.jetty.websocket.util.messages.ReaderMessageSink;
public class DecodedTextStreamMessageSink<T> extends AbstractDecodedMessageSink<Decoder.TextStream<T>>
public class DecodedTextStreamMessageSink<T> extends AbstractDecodedMessageSink.Stream<Decoder.TextStream<T>>
{
public DecodedTextStreamMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders) throws NoSuchMethodException, IllegalAccessException
public DecodedTextStreamMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
{
super(session, methodHandle, decoders);
}
@ -51,22 +51,18 @@ public class DecodedTextStreamMessageSink<T> extends AbstractDecodedMessageSink<
public void onStreamStart(Reader reader)
{
for (Decoder.TextStream<T> decoder : _decoders)
try
{
try
{
T obj = decoder.decode(reader);
_methodHandle.invoke(obj);
return;
}
catch (DecodeException e)
{
throw new CloseException(CloseReason.CloseCodes.CANNOT_ACCEPT.getCode(), "Unable to decode", e);
}
catch (Throwable t)
{
throw new CloseException(CloseReason.CloseCodes.CANNOT_ACCEPT.getCode(), "Endpoint notification error", t);
}
T obj = _decoder.decode(reader);
_methodHandle.invoke(obj);
}
catch (DecodeException e)
{
throw new CloseException(CloseReason.CloseCodes.CANNOT_ACCEPT.getCode(), "Unable to decode", e);
}
catch (Throwable t)
{
throw new CloseException(CloseReason.CloseCodes.CANNOT_ACCEPT.getCode(), "Endpoint notification error", t);
}
}
}

View File

@ -315,17 +315,16 @@ public class AvailableDecodersTest
{
// has duplicated support for the same target Type
Exception e = assertThrows(InvalidWebSocketException.class, () -> decoders.register(BadDualDecoder.class));
assertThat(e.getMessage(), containsString("Duplicate"));
assertThat(e.getMessage(), containsString("Multiple decoders with different interface types"));
}
@Test
public void testCustomDecoderRegisterOtherDuplicate()
{
// This duplicate of decoders is of the same interface type so will form a decoder list.
// Register DateDecoder (decodes java.util.Date)
decoders.register(DateDecoder.class);
// Register TimeDecoder (which also wants to decode java.util.Date)
Exception e = assertThrows(InvalidWebSocketException.class, () -> decoders.register(TimeDecoder.class));
assertThat(e.getMessage(), containsString("Duplicate"));
decoders.register(TimeDecoder.class);
}
}