Repository: eclipse/leshan (java)

Issue:
Client registration sends ct with invalid syntax
I tried the Leshan demo client towards a non-Leshan server and noticed that the client uses invalid syntax in the Register operation payload:

```
</>;ct=60 110 112 11542 11543;rt="oma.lwm2m",</1>;ver=1.1,</1/0>,</3>;ver=1.1,</3/0>,</6/0>,</3303>;ver=1.1,</3303/0>
```

The syntax for `ct` is given in RFC 7252 (CoAP) as follows:

```ABNF
ct-value =  cardinal
         /  DQUOTE cardinal *( 1*SP cardinal ) DQUOTE
```

Even in the absence of RFC 7252 there is an ABNF in RFC 6690 (CoRE link format) that does not allow space in unquoted values.

Leshan should quote the `ct` attribute if it contains spaces, like so:

```
</>;ct="60 110 112 11542 11543";rt="oma.lwm2m",</1>;ver=1.1,</1/0>,</3>;ver=1.1,</3/0>,</6/0>,</3303>;ver=1.1,</3303/0>
```

Leshan's own parser for this data is very forgiving (too forgiving, really) which is probably why this was not caught right away.

Pull request:
Fix invalid syntax for ct attribute (QUOTE must be used)
It aims to fix #1020.

Patch:
diff --git a/leshan-client-core/src/main/java/org/eclipse/leshan/client/util/LinkFormatHelper.java b/leshan-client-core/src/main/java/org/eclipse/leshan/client/util/LinkFormatHelper.java
index e250dae7..4c67b126 100644
--- a/leshan-client-core/src/main/java/org/eclipse/leshan/client/util/LinkFormatHelper.java
+++ b/leshan-client-core/src/main/java/org/eclipse/leshan/client/util/LinkFormatHelper.java
@@ -56,14 +56,20 @@ public final class LinkFormatHelper {
         attributes.put("rt", "\"oma.lwm2m\"");
         // serialize contentFormat;
         if (supportedContentFormats != null && !supportedContentFormats.isEmpty()) {
-            StringBuilder b = new StringBuilder();
-            Iterator<ContentFormat> iterator = supportedContentFormats.iterator();
-            b.append(iterator.next().getCode());
-            while (iterator.hasNext()) {
-                b.append(" ");
+            if (supportedContentFormats.size() == 1) {
+                attributes.put("ct", Integer.toString(supportedContentFormats.get(0).getCode()));
+            } else {
+                StringBuilder b = new StringBuilder();
+                b.append("\"");
+                Iterator<ContentFormat> iterator = supportedContentFormats.iterator();
                 b.append(iterator.next().getCode());
+                while (iterator.hasNext()) {
+                    b.append(" ");
+                    b.append(iterator.next().getCode());
+                }
+                b.append("\"");
+                attributes.put("ct", b.toString());
             }
-            attributes.put("ct", b.toString());
         }
 
         links.add(new Link(rootURL, attributes));
diff --git a/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/Registration.java b/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/Registration.java
index 798eac75..cfd33f71 100644
--- a/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/Registration.java
+++ b/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/Registration.java
@@ -659,19 +659,35 @@ public class Registration {
             Set<ContentFormat> supportedContentFormats = new HashSet<>();
 
             // add content format from ct attributes
-            String[] formats = ctValue.split(" ");
-            for (String codeAsString : formats) {
+            if (!ctValue.startsWith("\"")) {
                 try {
-                    ContentFormat contentformat = ContentFormat.fromCode(codeAsString);
-                    if (supportedContentFormats.contains(contentformat)) {
-                        LOG.warn("Duplicate Content format {} in ct={} attributes for registration {} of client {} ",
-                                codeAsString, supportedContentFormats, registrationId, endpoint);
-                    }
-                    supportedContentFormats.add(contentformat);
+                    supportedContentFormats.add(ContentFormat.fromCode(ctValue));
                 } catch (NumberFormatException e) {
                     LOG.warn(
-                            "Invalid supported Content format {} in ct={} attributes for registration {} of client {} ",
-                            codeAsString, supportedContentFormats, registrationId, endpoint);
+                            "Invalid supported Content format for ct attributes for registration {} of client {} :  [{}] is not an Integer",
+                            registrationId, endpoint, ctValue);
+                }
+            } else {
+                if (!ctValue.endsWith("\"")) {
+                    LOG.warn("Invalid ct value [{}] attributes for registration {} of client {} : end quote is missing",
+                            ctValue, registrationId, endpoint);
+                } else {
+                    String[] formats = Link.unquote(ctValue).split(" ");
+                    for (String codeAsString : formats) {
+                        try {
+                            ContentFormat contentformat = ContentFormat.fromCode(codeAsString);
+                            if (supportedContentFormats.contains(contentformat)) {
+                                LOG.warn(
+                                        "Duplicate Content format [{}] in ct={} attributes for registration {} of client {} ",
+                                        codeAsString, ctValue, registrationId, endpoint);
+                            }
+                            supportedContentFormats.add(contentformat);
+                        } catch (NumberFormatException e) {
+                            LOG.warn(
+                                    "Invalid supported Content format in ct={} attributes for registration {} of client {}: [{}] is not an Integer",
+                                    ctValue, registrationId, endpoint, codeAsString);
+                        }
+                    }
                 }
             }
 


Test patch:
diff --git a/leshan-client-core/src/test/java/org/eclipse/leshan/client/util/LinkFormatHelperTest.java b/leshan-client-core/src/test/java/org/eclipse/leshan/client/util/LinkFormatHelperTest.java
index cf0ef1b5..d540a30b 100644
--- a/leshan-client-core/src/test/java/org/eclipse/leshan/client/util/LinkFormatHelperTest.java
+++ b/leshan-client-core/src/test/java/org/eclipse/leshan/client/util/LinkFormatHelperTest.java
@@ -171,7 +171,7 @@ public class LinkFormatHelperTest {
         Link[] links = LinkFormatHelper.getClientDescription(objectEnablers, null,
                 Arrays.asList(ContentFormat.TLV, ContentFormat.JSON, ContentFormat.OPAQUE));
 
-        assertArrayEquals(Link.parse("</>;rt=\"oma.lwm2m\";ct=11542 11543 42,</6>".getBytes()), links);
+        assertArrayEquals(Link.parse("</>;rt=\"oma.lwm2m\";ct=\"11542 11543 42\",</6>".getBytes()), links);
     }
 
     @Test
diff --git a/leshan-integration-tests/src/test/java/org/eclipse/leshan/integration/tests/QueueModeTest.java b/leshan-integration-tests/src/test/java/org/eclipse/leshan/integration/tests/QueueModeTest.java
index edf15d38..5791dc7c 100644
--- a/leshan-integration-tests/src/test/java/org/eclipse/leshan/integration/tests/QueueModeTest.java
+++ b/leshan-integration-tests/src/test/java/org/eclipse/leshan/integration/tests/QueueModeTest.java
@@ -61,7 +61,7 @@ public class QueueModeTest {
         // Check client is well registered
         queueModeHelper.assertClientRegisterered();
         assertArrayEquals(Link.parse(
-                "</>;rt=\"oma.lwm2m\";ct=60 110 112 11542 11543,</1>;ver=1.1,</1/0>,</2>,</3>;ver=1.1,</3/0>,</2000/0>"
+                "</>;rt=\"oma.lwm2m\";ct=\"60 110 112 11542 11543\",</1>;ver=1.1,</1/0>,</2>,</3>;ver=1.1,</3/0>,</2000/0>"
                         .getBytes()),
                 queueModeHelper.getCurrentRegistration().getObjectLinks());
 
diff --git a/leshan-integration-tests/src/test/java/org/eclipse/leshan/integration/tests/RegistrationTest.java b/leshan-integration-tests/src/test/java/org/eclipse/leshan/integration/tests/RegistrationTest.java
index 8e357671..4a0dcdc0 100644
--- a/leshan-integration-tests/src/test/java/org/eclipse/leshan/integration/tests/RegistrationTest.java
+++ b/leshan-integration-tests/src/test/java/org/eclipse/leshan/integration/tests/RegistrationTest.java
@@ -85,7 +85,7 @@ public class RegistrationTest {
         // Check client is well registered
         helper.assertClientRegisterered();
         assertArrayEquals(Link.parse(
-                "</>;rt=\"oma.lwm2m\";ct=60 110 112 1542 1543 11542 11543,</1>;ver=1.1,</1/0>,</2>,</3>;ver=1.1,</3/0>,</2000/0>,</2000/1>"
+                "</>;rt=\"oma.lwm2m\";ct=\"60 110 112 1542 1543 11542 11543\",</1>;ver=1.1,</1/0>,</2>,</3>;ver=1.1,</3/0>,</2000/0>,</2000/1>"
                         .getBytes()),
                 helper.getCurrentRegistration().getObjectLinks());
 
@@ -111,7 +111,7 @@ public class RegistrationTest {
         // Check client is well registered
         helper.assertClientRegisterered();
         assertArrayEquals(Link.parse(
-                "</>;rt=\"oma.lwm2m\";ct=60 110 112 1542 1543 11542 11543,</1>;ver=1.1,</1/0>,</2>,</3>;ver=1.1,</3/0>,</2000/0>,</2000/1>"
+                "</>;rt=\"oma.lwm2m\";ct=\"60 110 112 1542 1543 11542 11543\",</1>;ver=1.1,</1/0>,</2>,</3>;ver=1.1,</3/0>,</2000/0>,</2000/1>"
                         .getBytes()),
                 helper.getCurrentRegistration().getObjectLinks());
 
@@ -262,7 +262,7 @@ public class RegistrationTest {
         assertNotNull(helper.getLastRegistration());
         assertEquals(additionalAttributes, helper.getLastRegistration().getAdditionalRegistrationAttributes());
         assertArrayEquals(Link.parse(
-                "</>;rt=\"oma.lwm2m\";ct=60 110 112 1542 1543 11542 11543,</1>;ver=1.1,</1/0>,</2>,</3>;ver=1.1,</3/0>,</2000/0>,</2000/1>"
+                "</>;rt=\"oma.lwm2m\";ct=\"60 110 112 1542 1543 11542 11543\",</1>;ver=1.1,</1/0>,</2>,</3>;ver=1.1,</3/0>,</2000/0>,</2000/1>"
                         .getBytes()),
                 helper.getCurrentRegistration().getObjectLinks());
     }
diff --git a/leshan-server-core/src/test/java/org/eclipse/leshan/server/registration/RegistrationTest.java b/leshan-server-core/src/test/java/org/eclipse/leshan/server/registration/RegistrationTest.java
index 136f338a..bc5cdd10 100644
--- a/leshan-server-core/src/test/java/org/eclipse/leshan/server/registration/RegistrationTest.java
+++ b/leshan-server-core/src/test/java/org/eclipse/leshan/server/registration/RegistrationTest.java
@@ -53,7 +53,7 @@ public class RegistrationTest {
 
     @Test
     public void test_object_links_with_ct_but_with_rt() {
-        Registration reg = given_a_registration_with_object_link_like("</>;ct=0 42 11543,</1/0>,</3/0>");
+        Registration reg = given_a_registration_with_object_link_like("</>;ct=\"0 42 11543\",</1/0>,</3/0>");
 
         // check root path
         assertEquals("/", reg.getRootPath());
@@ -76,10 +76,106 @@ public class RegistrationTest {
         assertTrue(availableInstances.containsAll(Arrays.asList(new LwM2mPath(1, 0), new LwM2mPath(3, 0))));
     }
 
+    @Test
+    public void test_object_links_with_ct_with_1_content_format_with_quote() {
+        Registration reg = given_a_registration_with_object_link_like("</>;ct=\"42\",</1/0>,</3/0>");
+
+        // check root path
+        assertEquals("/", reg.getRootPath());
+
+        // Ensure supported objects are correct
+        Map<Integer, String> supportedObject = reg.getSupportedObject();
+        assertEquals(2, supportedObject.size());
+        assertEquals(ObjectModel.DEFAULT_VERSION, supportedObject.get(1));
+        assertEquals(ObjectModel.DEFAULT_VERSION, supportedObject.get(3));
+
+        // Check Supported Content format
+        Set<ContentFormat> supportedContentFormats = reg.getSupportedContentFormats();
+        assertEquals(2, supportedContentFormats.size()); // 1 + 1 mandatory TLV content format for LWM2M v1.0
+        assertTrue(supportedContentFormats.containsAll(Arrays.asList(ContentFormat.TLV, ContentFormat.OPAQUE)));
+
+        // ensure available instances are correct
+        Set<LwM2mPath> availableInstances = reg.getAvailableInstances();
+        assertEquals(2, availableInstances.size());
+        assertTrue(availableInstances.containsAll(Arrays.asList(new LwM2mPath(1, 0), new LwM2mPath(3, 0))));
+    }
+
+    @Test
+    public void test_object_links_with_ct_with_1_content_format_without_quote() {
+        Registration reg = given_a_registration_with_object_link_like("</>;ct=42,</1/0>,</3/0>");
+
+        // check root path
+        assertEquals("/", reg.getRootPath());
+
+        // Ensure supported objects are correct
+        Map<Integer, String> supportedObject = reg.getSupportedObject();
+        assertEquals(2, supportedObject.size());
+        assertEquals(ObjectModel.DEFAULT_VERSION, supportedObject.get(1));
+        assertEquals(ObjectModel.DEFAULT_VERSION, supportedObject.get(3));
+
+        // Check Supported Content format
+        Set<ContentFormat> supportedContentFormats = reg.getSupportedContentFormats();
+        assertEquals(2, supportedContentFormats.size()); // 1 + 1 mandatory TLV content format for LWM2M v1.0
+        assertTrue(supportedContentFormats.containsAll(Arrays.asList(ContentFormat.TLV, ContentFormat.OPAQUE)));
+
+        // ensure available instances are correct
+        Set<LwM2mPath> availableInstances = reg.getAvailableInstances();
+        assertEquals(2, availableInstances.size());
+        assertTrue(availableInstances.containsAll(Arrays.asList(new LwM2mPath(1, 0), new LwM2mPath(3, 0))));
+    }
+
+    @Test
+    public void test_object_links_with_ct_with_several_without_quote() {
+        Registration reg = given_a_registration_with_object_link_like("</>;ct=0 42 11543,</1/0>,</3/0>");
+
+        // check root path
+        assertEquals("/", reg.getRootPath());
+
+        // Ensure supported objects are correct
+        Map<Integer, String> supportedObject = reg.getSupportedObject();
+        assertEquals(2, supportedObject.size());
+        assertEquals(ObjectModel.DEFAULT_VERSION, supportedObject.get(1));
+        assertEquals(ObjectModel.DEFAULT_VERSION, supportedObject.get(3));
+
+        // Check Supported Content format
+        Set<ContentFormat> supportedContentFormats = reg.getSupportedContentFormats();
+        assertEquals(1, supportedContentFormats.size()); // 1 mandatory TLV content format for LWM2M v1.0
+        assertTrue(supportedContentFormats.containsAll(Arrays.asList(ContentFormat.TLV)));
+
+        // ensure available instances are correct
+        Set<LwM2mPath> availableInstances = reg.getAvailableInstances();
+        assertEquals(2, availableInstances.size());
+        assertTrue(availableInstances.containsAll(Arrays.asList(new LwM2mPath(1, 0), new LwM2mPath(3, 0))));
+    }
+
+    @Test
+    public void test_object_links_with_ct_with_several_with_unclosed_quote() {
+        Registration reg = given_a_registration_with_object_link_like("</>;ct=\"0 42 11543,</1/0>,</3/0>");
+
+        // check root path
+        assertEquals("/", reg.getRootPath());
+
+        // Ensure supported objects are correct
+        Map<Integer, String> supportedObject = reg.getSupportedObject();
+        assertEquals(2, supportedObject.size());
+        assertEquals(ObjectModel.DEFAULT_VERSION, supportedObject.get(1));
+        assertEquals(ObjectModel.DEFAULT_VERSION, supportedObject.get(3));
+
+        // Check Supported Content format
+        Set<ContentFormat> supportedContentFormats = reg.getSupportedContentFormats();
+        assertEquals(1, supportedContentFormats.size()); // 1 mandatory TLV content format for LWM2M v1.0
+        assertTrue(supportedContentFormats.containsAll(Arrays.asList(ContentFormat.TLV)));
+
+        // ensure available instances are correct
+        Set<LwM2mPath> availableInstances = reg.getAvailableInstances();
+        assertEquals(2, availableInstances.size());
+        assertTrue(availableInstances.containsAll(Arrays.asList(new LwM2mPath(1, 0), new LwM2mPath(3, 0))));
+    }
+
     @Test
     public void test_object_links_with_default_rootpath() {
         Registration reg = given_a_registration_with_object_link_like(
-                "</>;rt=\"oma.lwm2m\";ct=0 42 11543,</1/0>,</3/0>");
+                "</>;rt=\"oma.lwm2m\";ct=\"0 42 11543\",</1/0>,</3/0>");
 
         // check root path
         assertEquals("/", reg.getRootPath());