Skip to content

Commit

Permalink
Introduce strict payload checks.
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Achim Kraus committed Jan 9, 2019
1 parent e3b1249 commit 532c03b
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,17 @@
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);

validateCharset(req, StandardCharsets.ISO_8859_1);
}

@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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 532c03b

Please sign in to comment.