From 532c03bbd8a8c7e6d9513fe3cbe47bd32834599b Mon Sep 17 00:00:00 2001 From: Achim Kraus Date: Tue, 8 Jan 2019 10:10:45 +0100 Subject: [PATCH] Introduce strict payload checks. Check, it message is intended to have payload and thorw exceptions, if not. Overwrite that check by mark a message to have unintended payload. Signed-off-by: Achim Kraus --- .../californium/core/coap/EmptyMessage.java | 10 ++++ .../californium/core/coap/Message.java | 59 ++++++++++++++++++- .../californium/core/coap/Request.java | 12 +++- .../network/serialization/DataParser.java | 3 + .../network/stack/Block1BlockwiseStatus.java | 3 + .../core/network/stack/BlockwiseLayer.java | 6 +- .../core/network/stack/BlockwiseStatus.java | 3 + .../californium/proxy/HttpTranslatorTest.java | 8 +-- .../californium/oscore/OptionJuggleTest.java | 2 +- 9 files changed, 91 insertions(+), 15 deletions(-) diff --git a/californium-core/src/main/java/org/eclipse/californium/core/coap/EmptyMessage.java b/californium-core/src/main/java/org/eclipse/californium/core/coap/EmptyMessage.java index be54030ff2..3e33b2bd4b 100644 --- a/californium-core/src/main/java/org/eclipse/californium/core/coap/EmptyMessage.java +++ b/californium-core/src/main/java/org/eclipse/californium/core/coap/EmptyMessage.java @@ -73,6 +73,16 @@ public int getRawCode() { return 0; } + /** + * {@inheritDoc} + * + * EMPTY messages are never intended to have payload! + */ + @Override + public boolean isIntendedPayload() { + return false; + } + /** * Create a new acknowledgment for the specified message. * diff --git a/californium-core/src/main/java/org/eclipse/californium/core/coap/Message.java b/californium-core/src/main/java/org/eclipse/californium/core/coap/Message.java index 1c6a8fb30d..eea449a39a 100644 --- a/californium-core/src/main/java/org/eclipse/californium/core/coap/Message.java +++ b/californium-core/src/main/java/org/eclipse/californium/core/coap/Message.java @@ -117,6 +117,9 @@ public abstract class Message { /** The payload of this message. */ private byte[] payload; + /** Marks this message to have payload even if this is not intended */ + private boolean unintendedPayload; + /** * Destination endpoint context. Used for outgoing messages. */ @@ -243,6 +246,41 @@ public Message setConfirmable(boolean con) { */ public abstract int getRawCode(); + /** + * Checks, if this message is intended to have payload. + * + * To be overwritten by subclass to provide a specific check. + * + * @return {@code true}, if message is intended to have payload + */ + public boolean isIntendedPayload() { + return true; + } + + /** + * Set marker for unintended payload. + * + * Enables to use payload with messages, which are not intended to have + * payload. + * + * @throws IllegalStateException if message is intended to have payload + */ + public void setUnintendedPayload() { + if (isIntendedPayload()) { + throw new IllegalStateException("Message is already intended to have payload!"); + } + unintendedPayload = true; + } + + /** + * Checks, if message is marked to have unintended payload. + * + * @return {@code true} if message is marked to have unintended payload + */ + public boolean isUnintendedPayload() { + return unintendedPayload; + } + /** * Gets the 16-bit message identification. * @@ -472,8 +510,15 @@ protected String getPayloadTracingString() { * * Provides a fluent API to chain setters. * - * @param payload the payload as sting + * @param payload the payload as string. {@code null} or a empty string are + * not considered to be payload and therefore not cause a + * IllegalArgumentException, if this message must not have + * payload. * @return this Message + * @throws IllegalArgumentException if this message must not have payload + * @see #isIntendedPayload() + * @see #isUnintendedPayload() + * @see #setUnintendedPayload() */ public Message setPayload(String payload) { if (payload == null) { @@ -489,10 +534,20 @@ public Message setPayload(String payload) { * * Provides a fluent API to chain setters. * - * @param payload the new payload + * @param payload the new payload. {@code null} or a empty array are not + * considered to be payload and therefore not cause a + * IllegalArgumentException, if this message must not have + * payload. * @return this Message + * @throws IllegalArgumentException if this message must not have payload + * @see #isIntendedPayload() + * @see #isUnintendedPayload() + * @see #setUnintendedPayload() */ public Message setPayload(byte[] payload) { + if (payload != null && payload.length > 0 && !isIntendedPayload() && !isUnintendedPayload()) { + throw new IllegalArgumentException("Message must not have payload!"); + } this.payload = payload; return this; } diff --git a/californium-core/src/main/java/org/eclipse/californium/core/coap/Request.java b/californium-core/src/main/java/org/eclipse/californium/core/coap/Request.java index dfa8f571bd..13102766c9 100644 --- a/californium-core/src/main/java/org/eclipse/californium/core/coap/Request.java +++ b/californium-core/src/main/java/org/eclipse/californium/core/coap/Request.java @@ -209,7 +209,15 @@ public boolean isMulticast() { /** * {@inheritDoc} * - * Required in Request to keep class for fluent API. + * GET and DELETE request are not intended to have payload. + */ + @Override + public boolean isIntendedPayload() { + return code != Code.GET && code != Code.DELETE; + } + + /** + * {@inheritDoc} */ @Override public Request setPayload(String payload) { @@ -219,8 +227,6 @@ public Request setPayload(String payload) { /** * {@inheritDoc} - * - * Required in Request to keep class for fluent API. */ @Override public Request setPayload(byte[] payload) { diff --git a/californium-core/src/main/java/org/eclipse/californium/core/network/serialization/DataParser.java b/californium-core/src/main/java/org/eclipse/californium/core/network/serialization/DataParser.java index 206dfecbad..c2db8c9df1 100644 --- a/californium-core/src/main/java/org/eclipse/californium/core/network/serialization/DataParser.java +++ b/californium-core/src/main/java/org/eclipse/californium/core/network/serialization/DataParser.java @@ -180,6 +180,9 @@ public static void parseOptionsAndPayload(DatagramReader reader, Message message message.getMID(), message.getRawCode(), message.isConfirmable()); } else { // get payload + if (!message.isIntendedPayload()) { + message.setUnintendedPayload(); + } message.setPayload(reader.readBytesLeft()); } } else { diff --git a/californium-core/src/main/java/org/eclipse/californium/core/network/stack/Block1BlockwiseStatus.java b/californium-core/src/main/java/org/eclipse/californium/core/network/stack/Block1BlockwiseStatus.java index 56e68af3fd..1c594056f1 100644 --- a/californium-core/src/main/java/org/eclipse/californium/core/network/stack/Block1BlockwiseStatus.java +++ b/californium-core/src/main/java/org/eclipse/californium/core/network/stack/Block1BlockwiseStatus.java @@ -127,6 +127,9 @@ public synchronized Request getNextRequestBlock() { // indicate overall body size to peer block.getOptions().setSize1(request.getPayloadSize()); } + if (request.isUnintendedPayload()) { + block.setUnintendedPayload(); + } int currentSize = getCurrentSize(); int from = num * currentSize; diff --git a/californium-core/src/main/java/org/eclipse/californium/core/network/stack/BlockwiseLayer.java b/californium-core/src/main/java/org/eclipse/californium/core/network/stack/BlockwiseLayer.java index 99c72bcd93..95939d3885 100644 --- a/californium-core/src/main/java/org/eclipse/californium/core/network/stack/BlockwiseLayer.java +++ b/californium-core/src/main/java/org/eclipse/californium/core/network/stack/BlockwiseLayer.java @@ -64,7 +64,6 @@ import java.util.concurrent.TimeUnit; import org.eclipse.californium.core.coap.BlockOption; -import org.eclipse.californium.core.coap.CoAP.Code; import org.eclipse.californium.core.coap.CoAP.ResponseCode; import org.eclipse.californium.core.coap.CoAP.Type; import org.eclipse.californium.core.coap.MessageObserver; @@ -1142,10 +1141,7 @@ private Block2BlockwiseStatus clearBlock2Status(KeyUri key, Block2BlockwiseStatu } private boolean requiresBlockwise(final Request request) { - boolean blockwiseRequired = false; - if (request.getCode() == Code.PUT || request.getCode() == Code.POST) { - blockwiseRequired = request.getPayloadSize() > maxMessageSize; - } + boolean blockwiseRequired = request.getPayloadSize() > maxMessageSize; if (blockwiseRequired) { LOGGER.debug("request body [{}/{}] requires blockwise transfer", request.getPayloadSize(), maxMessageSize); } diff --git a/californium-core/src/main/java/org/eclipse/californium/core/network/stack/BlockwiseStatus.java b/californium-core/src/main/java/org/eclipse/californium/core/network/stack/BlockwiseStatus.java index 72d57d6faf..f194482a5a 100644 --- a/californium-core/src/main/java/org/eclipse/californium/core/network/stack/BlockwiseStatus.java +++ b/californium-core/src/main/java/org/eclipse/californium/core/network/stack/BlockwiseStatus.java @@ -274,6 +274,9 @@ public final synchronized void assembleReceivedMessage(final Message message) { message.setOptions(new OptionSet(first.getOptions())); message.getOptions().removeBlock1(); message.getOptions().removeBlock2(); + if (!message.isIntendedPayload()) { + message.setUnintendedPayload(); + } message.setPayload(getBody()); } diff --git a/californium-proxy/src/test/java/org/eclipse/californium/proxy/HttpTranslatorTest.java b/californium-proxy/src/test/java/org/eclipse/californium/proxy/HttpTranslatorTest.java index e61ecba21a..e4f779b8be 100644 --- a/californium-proxy/src/test/java/org/eclipse/californium/proxy/HttpTranslatorTest.java +++ b/californium-proxy/src/test/java/org/eclipse/californium/proxy/HttpTranslatorTest.java @@ -32,8 +32,8 @@ public class HttpTranslatorTest { @Test - public void testGetHttpEntity() throws Exception { - Request req = new Request(Code.GET); + public void testPutHttpEntity() throws Exception { + Request req = new Request(Code.PUT); req.setPayload("payload"); req.getOptions().setContentFormat(MediaTypeRegistry.TEXT_PLAIN); @@ -41,8 +41,8 @@ public void testGetHttpEntity() throws Exception { } @Test - public void testGetHttpEntityWithJSON() throws Exception { - Request req = new Request(Code.GET); + public void testPutHttpEntityWithJSON() throws Exception { + Request req = new Request(Code.PUT); req.setPayload("{}"); req.getOptions().setContentFormat(MediaTypeRegistry.APPLICATION_JSON); diff --git a/oscore-cf/src/test/java/org/eclipse/californium/oscore/OptionJuggleTest.java b/oscore-cf/src/test/java/org/eclipse/californium/oscore/OptionJuggleTest.java index cf7319808d..49c9ca9010 100644 --- a/oscore-cf/src/test/java/org/eclipse/californium/oscore/OptionJuggleTest.java +++ b/oscore-cf/src/test/java/org/eclipse/californium/oscore/OptionJuggleTest.java @@ -138,7 +138,7 @@ public void testRealCodeRequest() { options.setAccept(accept); request.setOptions(options); - Code realCode = Code.GET; + Code realCode = Code.PUT; Request realed = OptionJuggle.setRealCodeRequest(request, realCode); OptionSet realOptions = realed.getOptions();