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

Why rejecting registration? #1229

Closed
Murata-Ruogu opened this issue Mar 31, 2022 · 13 comments
Closed

Why rejecting registration? #1229

Murata-Ruogu opened this issue Mar 31, 2022 · 13 comments
Labels
question Any question about leshan

Comments

@Murata-Ruogu
Copy link

Murata-Ruogu commented Mar 31, 2022

Our LWM2M client tried to register with payload:

</>;ct=110;ct=112,</1/0>,</2>,</3/0>,</4/0>,</5/0>,</7/0>,</15/0>,</16/0>

But got 4.00 response with reason:
Unable to parse </>;ct=110;ct=112,</1/0>,</2>,</3/0>,</4/0>,</5/0>,</7/0>,</15/0>,</16/0>

We were able to register before. It must be a new version issue. Is it related to #1020?

We do not have space for ct values, thus shouldn't need " around the value 110 or 112. Can you fix this?

@sbernard31 sbernard31 added the question Any question about leshan label Apr 1, 2022
@sbernard31
Copy link
Contributor

I tested your register payload and I get :

org.eclipse.leshan.core.link.LinkParseException: Unable to parse </>;ct=110;ct=112,</1/0>,</2>,</3/0>,</4/0>,</5/0>,</7/0>,</15/0>,</16/0>
	at org.eclipse.leshan.core.link.DefaultLinkParser.consumeLinkValue(DefaultLinkParser.java:121)
	at org.eclipse.leshan.core.link.DefaultLinkParser.parseCoreLinkFormat(DefaultLinkParser.java:80)
	at org.eclipse.leshan.core.link.lwm2m.DefaultLwM2mLinkParser.parseCoreLinkFormat(DefaultLwM2mLinkParser.java:116)
	at org.eclipse.leshan.core.link.DefaultLinkParserTest.parse_bug(DefaultLinkParserTest.java:43)
Caused by: java.lang.IllegalArgumentException: Cannot create attribute set with duplicates (attr: 'ct')
	at org.eclipse.leshan.core.link.attributes.AttributeSet.<init>(AttributeSet.java:41)
	at org.eclipse.leshan.core.link.Link.<init>(Link.java:64)
	at org.eclipse.leshan.core.link.DefaultLinkParser.consumeLinkValue(DefaultLinkParser.java:119)
	... 29 more

So it fails because you are using ct twice.
The working way to send 2 values for ct attribtue is : </>;ct="110 112"

This is because we consider that same attribute can not be used twice for same Link.
Maybe this is wrong assumption but I never see any example of attribute used twice.
@weinholt, do you have some hint about this ?

We were able to register before. It must be a new version issue. Is it related to #1020?

With #1020 raised, we try to be more strict and respectful of IETF RFC but I believed that we never tolerate this ☝️. (I will double check this later)

Anyway is it possible to you to use this notation : </>;ct="110 112" ?
If not, a shortterm workaround is to implement your own LinkParser (see LeshanServerBuilder.setLinkParser(LwM2mLinkParser)) or to add a request interceptor to hack the payload (See #1103)

@sbernard31
Copy link
Contributor

I just tested with Leshan v1.4.0, the payload </>;ct=110;ct=112,</1/0>,</2>,</3/0>,</4/0>,</5/0>,</7/0>,</15/0>,</16/0> is accepted. I mean no exception is raised but the first ct attribute is lost.

And so this is like if </>;ct=112,</1/0>,</2>,</3/0>,</4/0>,</5/0>,</7/0>,</15/0>,</16/0> was raised.
I think this previous behavior is worst.

@Murata-Ruogu
Copy link
Author

Can you point out where the spec says you cannot use ct twice? Can you add support for that?

@sbernard31
Copy link
Contributor

There is 2 questions here :

  1. Is it allowed by the specification or not ?
  2. Should we change the current behavior.

About 1), as I said, I don't see any example where attribute is used twice and nothing we let me know it is allowed.
In the rfc7252#section-7.2.1we can read :

The Content-Format code attribute MAY include a space-separated sequence of Content-Format
codes, indicating that multiple content-formats are available.

Which seems to let know that if several Content-Format is used the ct="110 112" notation must be used.

But I agree there is no strong hint that it is not allowed.

About 2), we currently used Map indexed by attribute name (e.g. ct) to store attribute, so changing this will be impacting and so I could consider to change this only if we are sure this is allowed by RFC. I guess maybe we should ask some question about it at IETF, or maybe wait for some feedback from @weinholt.

Anyway, that means unfortunately, I would not provide a quick change about this soon.

So at short term my advice is to change your device behavior OR use the workaround I provide above ☝️ .

@weinholt
Copy link

weinholt commented Apr 6, 2022

At first it would seem that </>;ct=110;ct=112 is wrong, it should be </>;ct="110 112". That is how RFC7252 says multiple content formats are written. But writing ct multiple times is syntactically valid according to the ABNF in RFC6690. So if it is syntactically valid, what are the semantics?

In RFC6690 (CoRE Link Format) there are some attributes that can't appear more than once, and they felt the need to explicitly point this out, like here for the rt attribute:

The Resource Type attribute MUST NOT appear more than once in a link.

So is there a similar restriction on the ct attribute? RFC7252 (CoAP) defines this attribute and does not mention such a restriction. There happens to be an erratum on this RFC for the missing restriction on multiple ct attributes: https://www.rfc-editor.org/errata/eid5078. The status of the erratum is "reported" rather than "verified", so I guess nobody has had the time to look at it yet.

Another interesting RFC in this context is RFC5988 (Web Linking), which CoRE Link Format is based on. Some of the attributes in RFC5988 have this sort of text:

The "rel" parameter MUST NOT appear more than once in a given link-value; occurrences after the first MUST be ignored by parsers.

This explicitly tells us how to interpret multiple rel parameters, if that was our problem.

It's interesting that the RFCs got more lax in their language as time went on. RFC5988 has both the "MUST NOT appear" and "MUST be ignored" text. It is followed by RFC6690 that forgets about the "MUST be ignored" part, and that is followed by RFC7252 that also forgets the "MUST NOT appear", at which point nothing is left of the original MUSTs.

AFAICT, none of these RFCs say that multiple occurrences should signal a parser error.

(The parser I worked with in #1020 does not reject the multiple ct attributes. That parser basically represents attributes as a list of key-value pairs. So it doesn't lose any information when parsing the string containing the links, and it can also later reconstruct the same string. But that is just because it's a generic parser and serializer for the CoRE Link Format. The server implementation uses that parser but then just picks the first ct attribute it sees. So in practice it wouldn't matter for that particular server implementation if the parser kept or dropped the latter ct attributes. Using the equivalent of a Map would be fine.)

My opinion is that Leshan should not reject </>;ct=110;ct=112 because there is nothing syntactically wrong with it. How should it be interpreted? I can in principle see nothing technically wrong with ignoring every occurrence of ct after the first. It would be in line with how RFC5988 would handle it, and I understand that it's how previous Leshan versions did it.

Perhaps the LwM2M spec should be updated to say what the correct behavior is. There's already a "MAY" in there that lets servers get away with parsing lwm2m=1.0,</0/0>;ssid=101,</0/1/>,</0/2>;ssid=102, which is completely missing the </>; part.

@sbernard31
Copy link
Contributor

@weinholt, thx a lot for the detailed answer, that really helps 🙏

I will try to look if there is a not so impacting way to use list instead of map but I guess this will not be the case.
Else I can consider to just keep the first value.

I can in principle see nothing technically wrong with ignoring every occurrence of ct after the first. It would be in line with how RFC5988 would handle it, and I understand that it's how previous Leshan versions did it.

Not really, it seems the previous Leshan version was keeping the last occurence (Note this was not really intended)

Perhaps the LwM2M spec should be updated to say what the correct behavior is.

I will try to get some hints from OMA : OpenMobileAlliance/OMA_LwM2M_for_Developers#544

There's already a "MAY" in there that lets servers get away with parsing lwm2m=1.0,</0/0>;ssid=101,</0/1/>,</0/2>;ssid=102, which is completely missing the </>; part.

You mean the LWM2M spec allow something which is not allowed by the ABNF in RFC6690 ? 😨

@sbernard31
Copy link
Contributor

@Murata-Ruogu how do you deal with this problem for now ?

@Murata-Ruogu
Copy link
Author

We are relying on our vendor to decide what to do. But for now, I am using other servers. Thanks.

@weinholt
Copy link

weinholt commented Apr 7, 2022

You mean the LWM2M spec allow something which is not allowed by the ABNF in RFC6690 ?

@sbernard31 Yes, there is an exception, but it's only for the Bootstrap-Discover response. I don't think it applies to the registration, and supporting it is not mandatory. From OMA-TS-LightweightM2M_Core-V1_2-20201110-A:

Note: In previous versions of this document, the LwM2M Enabler Version link parameter was not associated to any
URI-Reference in the examples (e.g. "lwm2m=1.0,</0/0>;ssid=101,</0/1/>,</0/2>;ssid=102"). Thus, the provided
examples were not in conformance with the ABNF of [RFC6690]. LwM2M Servers MAY accept this particular error
in the LwM2M Client's response to the "Bootstrap-Discover" operation.

@sbernard31
Copy link
Contributor

Yes, there is an exception.

This kind of exception does not really help re-usability and LWM2M developers ... 😞

@Murata-Ruogu
Copy link
Author

I was notified our vendor would do something about it in the client side. So you do not need to make any change for now. Thanks.

@sbernard31
Copy link
Contributor

OMA seems to confirm that the right way to go is </>;ct="110 112"
See : OpenMobileAlliance/OMA_LwM2M_for_Developers#544 (comment)

@sbernard31
Copy link
Contributor

This sounds to confirm that it is OK to not support several ct : core-wg/corrclar#4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Any question about leshan
Projects
None yet
Development

No branches or pull requests

3 participants