okleine / nCoAP

Java implementation of the CoAP protocol using netty
BSD 3-Clause "New" or "Revised" License
179 stars 57 forks source link

A CoapRequest cannot conain a Uri-Host #31

Closed boldt closed 7 years ago

boldt commented 8 years ago

Cite of section 5.4.4. Default Values:

Options may be defined to have a default value.  If the value of an
option is intended to be this default value, the option SHOULD NOT be
included in the message.

SHOULD NOT means, that a default value could still be included in the message.

Cite of section 5.10.1. Uri-Host, Uri-Port, Uri-Path, and Uri-Query:

The default value of the Uri-Host Option is the IP literal
representing the destination IP address of the request message.

This means, that the destination IP (which is the default value) could still be included in the message.

Looking into the implementation of OptionValue:

        if (optionNumber == URI_HOST) {
            String hostName = new String(value, CoapMessage.CHARSET);
            if (hostName.startsWith("[") && hostName.endsWith("]"))
                hostName = hostName.substring(1, hostName.length() - 1);

            if (InetAddresses.isInetAddress(hostName))
                return true;
        }

This means, any IP (e.g., 0.0.0.0, 192.168.1.100, ...) is interpreted as a default value, which is wrong.

Because of this, the following example, which must be RFC compliant, throws an exception:

URI webserviceURI = new URI("coap://" + HOST + ":" + PORT + "/registry");
CoapRequest coapRequest = new CoapRequest(MessageType.Name.CON, MessageCode.Name.POST, webserviceURI, false);
coapRequest.addOption(OptionValue.Name.URI_HOST, new StringOptionValue(OptionValue.Name.URI_HOST, HOST));
okleine commented 8 years ago

Please note, that the capitalized phrase "SHOULD NOT" in RFCs is not an emphasis but has a well-defined meaning according to RFC 2119:

This phrase, or the phrase "NOT RECOMMENDED" mean that there may exist valid reasons in particular circumstances when the particular behavior is acceptable or even useful, but the full implications should be understood and the case carefully weighed before implementing any behavior described with this label.

Thus, this behavior is as it is on purpose. But maybe you can convince me of the opposite. Do you know any use-case that makes it necessary to include an IP address as an explicit option value?

Besides, the JavaDoc of the method addOption says, that it is a method for internal use only. There is a seperate method for each supported option. All (necessary) target URI resp. Proxy URI options are supposed to be automatically set within the constructor of CoapRequest.

boldt commented 8 years ago

First, the RFC 7252 gives an example in 5.10.1. Uri-Host, Uri-Port, Uri-Path, and Uri-Query:

Explicit Uri-Host and Uri-Port Options are typically used when an endpoint 
hosts multiple virtual servers.

In our case, we try to use CoAP on the NodeMCU, which shall talk to the SimpleCoapServer. We use the following CoAP module, provided by NodeMCU: http://nodemcu.readthedocs.io/en/dev/en/modules/coap/

This library can be used as follows to create a request:

cc = coap.Client()
cc:post(coap.NON, "coap://192.168.18.100:5683/", "Hello")

Here, the library adds the Uri-Host option to the request, which is okay from my interpretation of the RFC. Using the SimpleCoapServer, the option cannot be parsed by OptionValue (used by StringOptionValue; used by the CoapMessageDecoder). Thus, the server rejetcs the request with a 4.02 Bad option (see screenshot):

nodemcu

okleine commented 8 years ago

I see the point for decoding, i.e. inbound messages. I'll have a look at this maybe later today.

For encoding I still can't see any reason to change the actual behavior. If the addressed server is virtual there are two options:

First, the virtual servers have different IPs. In that case, there is no reason to explicitly include it as host option value.

Second option is, the virtual servers have different DNS names. In this case, the DNS name is included as host option value, anyway.

Do you agree?

okleine commented 8 years ago

The new constructors of OptionValue, StringOptionValue and UintOptionValue now have a parameter that allows to create default values. This is the new default behaviour during message decoding to avoid situations like the one you mentioned (the exception while decoding a kind of malformed inbound requests).

For outbound requests, the default behaviour is mostly unchanged, i.e. an IP address as host option or port number 5683 as port option both can not be set via CoapRequest constructor. However, you may use something like this (note the true as third parameter of the constructor):

String host = "10.20.30.40";
StringOptionValue defaultValue = new StringOptionValue(OptionValue.URI_HOST, host, true));
coapRequest.addOption(OptionValue.URI_HOST, defaultValue)

Could you please verify if this works for you?