Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SenMLEncoder can not encode Node for multi instance resource with too long resource or resource instance id #1024

Closed
bocajim opened this issue Jun 8, 2021 · 12 comments
Labels
bug Dysfunctionnal behavior client Impact LWM2M client server Impact LWM2M server

Comments

@bocajim
Copy link

bocajim commented Jun 8, 2021

Hello,

I apologize but I do not have the resources to submit a pull request, but there are 2 bugs in the LWM2M 1.1 implementation that need to be fixed:

  1. Define an object with a String resource with multiple values, define 2 values with resource array ids 10 and 20, and try to read them using senml_cbor. You will get an error. The bug is in LwM2mNodeSenMLEncoder. I fixed it in the patch attached, but I'm not sure this is the best way to fix it, please review.

  2. handlePATCH was incorrectly named handleIPatch which caused all composite writes to fail. See attached patch.

Thanks.
leshan_fixes.txt

@sbernard31
Copy link
Contributor

🙏 Thx for reporting this. I will look at this.

Which version are you using ?

I apologize but I do not have the resources to submit a pull request

What do you mean exactly ? 🤔

@sbernard31
Copy link
Contributor

About 2.

handlePATCH was incorrectly named handleIPatch which caused all composite writes to fail. See attached patch.

I just look at the specification transport-v1.1§6.4.4 - Device Management and Service EnablementInterface and it seems this is not an issue because iPatch MUST be used :

Operation CoAP Method and Content Formats Path Success Failure
Write-Composite iPATCH Content Format: SenML CBOR or SenML JSON (see [LwM2M-CORE]) URI path and a new value for each of resource to be written to is provided in request payload 2.04 Changed 4.00 Bad Request, 4.01 Unauthorized, 4.04 Not Found, 4.05 Method Not Allowed, 4.06 Not Acceptable

What makes you think PATCH must be used instead ?

(I will look at 1. now)

@sbernard31
Copy link
Contributor

About 1.

I tried to reproduce it, writting a integration test in WriteMultiValueTest :

@Test
public void can_write_object_resource_instance() throws InterruptedException {

    Map<Integer, String> values = new HashMap<>();
    values.put(10, "value10");
    values.put(20, "value20");

    // write multi instance
    WriteResponse response = helper.server.send(helper.getCurrentRegistration(),
            new WriteRequest(contentFormat, IntegrationTestHelper.TEST_OBJECT_ID, 0,
                    IntegrationTestHelper.STRING_RESOURCE_INSTANCE_ID, values, Type.STRING));

    // Verify Write result
    assertEquals(ResponseCode.CHANGED, response.getCode());
    assertNotNull(response.getCoapResponse());
    assertThat(response.getCoapResponse(), is(instanceOf(Response.class)));

    // read multi instance
    ReadResponse readResponse = helper.server.send(helper.getCurrentRegistration(), new ReadRequest(contentFormat,
            IntegrationTestHelper.TEST_OBJECT_ID, 0, IntegrationTestHelper.STRING_RESOURCE_INSTANCE_ID));

    // verify result
    assertEquals(CONTENT, readResponse.getCode());
    assertContentFormat(contentFormat, readResponse);

    LwM2mMultipleResource resourceInstance = (LwM2mMultipleResource) readResponse.getContent();
    assertEquals("value10", resourceInstance.getInstance(10).getValue());
    assertEquals("value20", resourceInstance.getInstance(20).getValue());
}

and it works for me.

SenMLCBOR payload in the response of Read Request looks like : 82a300623230036776616c75653230216b2f323030302f302f31302fa200623130036776616c75653130
I check with cbor.me and seems OK to me.

Maybe you could share your payload or a wireshark capture this way I could look at this ?
And tell if you are using Leshan at client, server or both side.

@sbernard31 sbernard31 added the question Any question about leshan label Jun 8, 2021
@bocajim
Copy link
Author

bocajim commented Jun 8, 2021

Regarding .1,

Here is the SenML CBOR for my write request:
(no leading slash) 82a2006d3636362f302f313030322f31300363414243a2006d3636362f302f313030322f32300363444546
(leading slash) 82a2006e2f3636362f302f313030322f31300363414243a2006e2f3636362f302f313030322f32300363444546
(it is not using the base name, it fully encodes the resources)

Regarding .2, I think you are correct, it looks like iPATCH is the correct method.

@sbernard31
Copy link
Contributor

sbernard31 commented Jun 9, 2021

Regarding #2, I think you are correct, it looks like iPATCH is the correct method.

Good we agree on that 🙂

Regarding .1,

I'm lost as I'm not able to reproduce this...
You said the error is in LwM2mNodeSenMLEncoder on READ request, Encoder is used for READ only at client side, so I guess your issue is about Leshan Client ? I should be able to reproduce with the test above ...I do not understand ...

So please give me some input that I could understand.
Which version of Leshan are you using ?
Are you using Leshan at client, server or both side ?
Where is the issue client or server side ?
How do you get those SenML CBOR payloads ? are they generated by Leshan ?
If yes, I do not understand how ? It doesn't look like what I see when I try to reproduce ...
Or maybe this is generated by another client but if this is the case how Encoder could be involved on a READ request at server side ...
If this is done by Leshan you could eventually share a snippet of code or better a Test like this ?

@bocajim
Copy link
Author

bocajim commented Jun 17, 2021

Sorry for the late reply...

Yes, I'm using the Leshan client, I'm using the most current code on master (SHA 7c5a398).

I repeated the test using the leshan client and leshan server on the master branch, I have a custom object with a multi-resource string using indexes 10/20, attached the object here.

When I use the leshan server, set the encoding to SENML_CBOR and try to read the MV stirng resource, I get the following error:

2021-06-17 17:14:20,339 ERROR LwM2mCoapResource - Exception while handling request [CON-GET MID=30688, Token=C0209FF39E11B708, OptionSet={"Uri-Path":["666","0","1002"], "Accept":"application/senml+cbor"}, no payload] on the resource /666 from Identity /127.0.0.1:5783[unsecure]
org.eclipse.leshan.core.node.LwM2mNodeException: Invalid resource id 100210, It MUST be an unsigned int.
at org.eclipse.leshan.core.node.LwM2mNodeUtil.validateResourceId(LwM2mNodeUtil.java:84)
at org.eclipse.leshan.core.node.LwM2mNodeUtil.validatePath(LwM2mNodeUtil.java:107)
at org.eclipse.leshan.core.node.LwM2mPath.validate(LwM2mPath.java:129)
at org.eclipse.leshan.core.node.LwM2mPath.(LwM2mPath.java:110)
at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLEncoder$InternalEncoder.addSenMLRecord(LwM2mNodeSenMLEncoder.java:262)
at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLEncoder$InternalEncoder.lwM2mResourceToSenMLRecord(LwM2mNodeSenMLEncoder.java:237)
at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLEncoder$InternalEncoder.visit(LwM2mNodeSenMLEncoder.java:203)
at org.eclipse.leshan.core.node.LwM2mMultipleResource.accept(LwM2mMultipleResource.java:216)
at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLEncoder.encode(LwM2mNodeSenMLEncoder.java:66)
at org.eclipse.leshan.core.node.codec.DefaultLwM2mNodeEncoder.encode(DefaultLwM2mNodeEncoder.java:148)
at org.eclipse.leshan.client.californium.object.ObjectResource.handleGET(ObjectResource.java:164)
at org.eclipse.californium.core.CoapResource.handleRequest(CoapResource.java:226)
at org.eclipse.leshan.core.californium.LwM2mCoapResource.handleRequest(LwM2mCoapResource.java:51)
at org.eclipse.californium.core.server.ServerMessageDeliverer.deliverRequest(ServerMessageDeliverer.java:106)
...

You can see that the read request is object 666, instance 0, resource 1002, but leshan is interpreting the resource instance values as 100210 and 100220, which is incorrect.

TestData.java.txt

@sbernard31
Copy link
Contributor

I will try this. Thx for this clarification.

@sbernard31
Copy link
Contributor

sbernard31 commented Jun 18, 2021

OK I succeed to reproduce it. 🎊
My previous test didn't succeed because Instance ID was to small and this bug doesn't create invalid payload. The invalid path (/666/0/100210] created is just used for logging ... 😬

I don't know if you just missed to share with me the model of your 666 object, but keep in mind that if you don't use a model for your object (at client and server side) you will probably face some issue

@bocajim
Copy link
Author

bocajim commented Jun 18, 2021

Sorry, here is the XML file
666.xml.txt

sbernard31 added a commit that referenced this issue Jun 18, 2021
This bug affected SenML CBOR and SenML JSON for multi instance resource
where resource id and or resource instance id is pretty long.
@sbernard31 sbernard31 added bug Dysfunctionnal behavior client Impact LWM2M client server Impact LWM2M server and removed question Any question about leshan labels Jun 18, 2021
@sbernard31 sbernard31 changed the title LWM2M 1.1 issues SenMLEncoder can not encode Node for multi instance resource with too long resource or resource instance id Jun 18, 2021
@sbernard31
Copy link
Contributor

@bocajim could you check if #1030 fix the issue for you ? 🙏

@bocajim
Copy link
Author

bocajim commented Jun 18, 2021

Tested the PR and it looks good on my side now. Thanks!

@sbernard31
Copy link
Contributor

Thx to you for taking time to reporting this 🙏 ! This helps a lot.

(This is now integrated in master)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Dysfunctionnal behavior client Impact LWM2M client server Impact LWM2M server
Projects
None yet
Development

No branches or pull requests

2 participants