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

Backwards compatibility with quoted object versions in CoRE Links #1270

Closed
adamsero opened this issue Jun 21, 2022 · 8 comments
Closed

Backwards compatibility with quoted object versions in CoRE Links #1270

adamsero opened this issue Jun 21, 2022 · 8 comments
Labels
question Any question about leshan Wait for answer Issue blocked waiting user feedback

Comments

@adamsero
Copy link
Contributor

Following the recent CoRE Link refactoring in #1197, we lost some backwards compatibility features, namely the server's ability to accept quoted object version values during Register and Discover. As an example, the following link fragment will not be accepted: </3>;ver="1.0" since it uses quotes around the version value (An error will be thrown: Invalid lwm2m version "1.0" in </3>;ver="1.0" : version should not be quoted).

Lack of quotes is compliant with LwM2M 1.1, but in LwM2M 1.0 it was not defined and there are multiple implementations from well established vendors that use quotes, since in LwM2M 1.0 it didn't make a difference. To make Leshan compatible with devices that use the aforementioned standard, it would be useful to accept both version value styles, with and without quotes.

@sbernard31 sbernard31 added the question Any question about leshan label Jun 21, 2022
@sbernard31
Copy link
Contributor

There 2 kind of attribute version. The lwm2m version and the object version.

For the first one there no issue as we know the LWM2M version used.
See : LwM2mVersionAttributeModel

For the object one, this is more complication has currently with the current code, we don't have access to the lwm2m version used.
By default this is not tolerated. The reason is explained in ObjectVersionAttributeModel.

But this is a real question about :
1. what should be the default behavior
2. or how much this should be easy to change the code to tolerate this
3. or should we try to change the code in a way we know LWM2M version when we parse attribute.

To make Leshan compatible with devices that use the aforementioned standard, it would be useful to accept both version value styles, with and without quotes.

So currently this is not the default behavior but this is possible. (maybe not easy enough to set ?)

Why be strict by default, In a general way I try to explain this at (#1103 (comment) (see answer to ""how should we do that ?"")
I should probably create a kind of documentation about this 🤔

For this case, in particular we can discuss about it.

By the way you find a little message issue 🕵️ , this should be

---Invalid lwm2m version "1.0" in </3>;ver="1.0" : version should not be quoted
+++Invalid object version "1.0" in </3>;ver="1.0" : version should not be quoted

@adamsero
Copy link
Contributor Author

I think we should be more relaxed about that particular case and tolerate both quoted and unquoted object versions by default. In terms of adapting the code for this purpose, it couldn't be easier; you only need to change one line of code in ObjectVersionAttributeModel class:

protected boolean tolerateQuote() {
        return false; //<-- change to true
}

If we don't do that, we might actually lose a substantial number of big clients and vendors since Leshan would no longer be compatible. I know it's bending your rules a little bit, but please consider this option.

@sbernard31
Copy link
Contributor

My problem with tolerating this by default is that will hide potential implementation issue of LWM2M v1.1. 😞
Generally I prefer to show the error first, to let user aware of the issue, then have a way support the "bad" behavior.

it couldn't be easier; you only need to change one line of code

I code it to let people changing it easy if needed. 🙂

If we don't do that, we might actually lose a substantial number of big clients and vendors since Leshan would no longer be compatible

I'm not sure to understand 🤔.
It would just be not compatible out of the box but we support this kind of device with some (more or less easy) tweaking.

Or I missed something ?

@adamsero
Copy link
Contributor Author

The issue is that since M7, Leshan is no longer compatible out of the box with devices that use LwM2M 1.0, since the specification for that version didn't strictly describe the quote behavior. There actually are a lot of devices on the market and already in production that use LwM2M 1.0 (and quoted object versions), so by becoming more and more strict we lose compatibility with them. IMO we should be more permissive at least in this instance and tolerate quotes.

@sbernard31
Copy link
Contributor

sbernard31 commented Jul 18, 2022

The issue is that since M7, Leshan is no longer compatible out of the box with devices that use LwM2M 1.0, since the specification for that version didn't strictly describe the quote behavior.

I understand but as I explained above, there is no easy way for now to be strict for 1.1 and flexible for 1.0.

There actually are a lot of devices on the market and already in production that use LwM2M 1.0 (and quoted object versions), so by becoming more and more strict we lose compatibility with them.

Not so true, we don't lost compatibility we just lost it out of the box. So all those devices can be used with Leshan.

IMO we should be more permissive at least in this instance and tolerate quotes.

In my experience, being flexible by default just hide the issues and then we find more more implementation with bad behavior.

So I feel this is better to make life a bit more complicated for people we want the quote support for LWM2M v1.0
And make life easier to people which want to support LWM2M v1.1

Note also that there is lot of specification issues which was raised about LWM2M v1.0 and was only fixed in LWM2M v1.1.
In that case, I feel that OMA just missed to defined this and there is bugs in examples, then they fix it in LWM2M v1.1.

So even for a LWM2M v1.0 device, I strongly advice to use quote !

Anyway to be more clear :

  • I really really don't like the idea to tolerate quote for LWM2M 1.1 by default.
  • I'm not against taking more time to find a way to have different behavior for v1.0 and v1.1 (but after Add a way to add more transport Layer  #1025)
  • I'm not against providing a more easy way to change the default behavior (e.g add CLI option in leshan-server-demo)

@sbernard31
Copy link
Contributor

sbernard31 commented Jul 19, 2022

About

By the way you find a little message issue , this should be

---Invalid lwm2m version "1.0" in </3>;ver="1.0" : version should not be quoted
+++Invalid object version "1.0" in </3>;ver="1.0" : version should not be quoted

I fixed it in #1290.

@sbernard31
Copy link
Contributor

@JaroslawLegierski nobody complain about that for while now.

So I don't know if we should try to fix that OR if finally everybody use quote now ?

@sbernard31 sbernard31 added the Wait for answer Issue blocked waiting user feedback label Jan 20, 2025
@JaroslawLegierski
Copy link
Contributor

@JaroslawLegierski nobody complain about that for while now.

So I don't know if we should try to fix that OR if finally everybody use quote now ?

Yes - we can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Any question about leshan Wait for answer Issue blocked waiting user feedback
Projects
None yet
Development

No branches or pull requests

3 participants