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

On 32-bit systems UINT32_MAX properties aren't handled correctly #1336

Closed
Zephyrus29 opened this issue Mar 7, 2023 · 5 comments
Closed

On 32-bit systems UINT32_MAX properties aren't handled correctly #1336

Zephyrus29 opened this issue Mar 7, 2023 · 5 comments
Milestone

Comments

@Zephyrus29
Copy link

** Description **
MQTT properties such as the session expiry are not handled correctly if they're set to UINT32_MAX. This is due to them being handled as an int instead of an unsigned int.

To Reproduce
On a 32-bit system set MQTTPROPERTY_CODE_SESSION_EXPIRY_INTERVAL to UINT32_MAX
Record the session expiry from the broker, it will be 0xFF

Expected behavior
Should pass up 0xFFFFFFFF instead of 0xFF

Additional context
Should use uint32_t instead of int in the properties struct

@icraggs
Copy link
Contributor

icraggs commented Mar 9, 2023

Which structure? The MQTTProperty structure in MQTTProperties.h has them unsigned.

However, the MQTTProperties_getNumericValue function returns an int rather than an unsigned int.

@Zephyrus29
Copy link
Author

I see this in MQTTProperty_write in writeInt4.

As that takes an int, it'll end up being -1 in writeInt4 so (-1/ 16777216) is 0 and same with the other functions in there.

I think either change that to unsigned int or to change it to (anInt >> 24) & 0xFF and so on for the rest of the values

@zhizhi-dev
Copy link
Contributor

I have fixed this issue
#1327

@icraggs
Copy link
Contributor

icraggs commented Apr 27, 2023

So I think that writeInt4 and readInt4 should definitely be changed to take and return unsigned ints respectively. I'll make that change.

@Zephyrus29 can you try out the PR from @zhizhi-dev and see if it works for you? Thanks.

@icraggs
Copy link
Contributor

icraggs commented Oct 9, 2023

I've reverted Pr #1327 because of errors created.

writeInt4 and readInt4 have been changed to take and return an unsigned int, rather than an int, so that might help. That might work - if anyone can try it out I'd appreciate it (lack of 32-bit environment easy to get to).

icraggs added a commit that referenced this issue Oct 9, 2023
@icraggs icraggs closed this as completed Oct 18, 2023
icraggs added a commit that referenced this issue Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants