coap: block_number in coap_get_block1_option() is too small

[yashi]

Describe the bug

RFC 7959 states that the Block Number can be more than 8 bits.

           0
           0 1 2 3 4 5 6 7
          +-+-+-+-+-+-+-+-+
          |  NUM  |M| SZX |
          +-+-+-+-+-+-+-+-+

           0                   1
           0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
          +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
          |          NUM          |M| SZX |
          +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

           0                   1                   2
           0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3
          +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
          |                   NUM                 |M| SZX |
          +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                       Figure 1: Block Option Value

However, block_number in coap_get_block1_option() and coap_get_block2_option() is declared as uint8_t.

To Reproduce
Try samples/net/sockets/coap_server/ with coap_get_option_int() replaced with coap_get_block1_option()

Expected behavior

block_number to be unsigned int.

Impact
Block number is truncated when using the APIs.

Environment (please complete the following information):

• OS: Linux Debian Sid
• Toolchain: Zephyr SDK v0.16.6
• Commit SHA: v3.7.0-rc3-25-gcb17f5f5a4af

[boaks]

Fixing that by changing the pointer type will be an API break.
Maybe it's easier to document the limit and reference to use the coap_block_context variants for larger transfers?

[rlubos]

Fixing that by changing the pointer type will be an API break. Maybe it's easier to document the limit and reference to use the coap_block_context variants for larger transfers?

The CoAP API is still considered "unstable", and we'll announce this rather trivial change in the migration guide. I'd personally prefer just to fix this instead having to document limitations of not-well-thought API functions.

[pdgendt]

I agree, especially since the coap_append_block[1|2]_option functions allow to set these values.

[boaks]

Just in the case, the API gets changed, adding the "more" flag is also helpful.

[rlubos]

Just in the case, the API gets changed, adding the "more" flag is also helpful.

That's a good point, it seems that coap_get_block2_option() has purposely limited the functionality to requests only:

Ignore the has_more flag as it should always be zero on queries.

@SeppoTakalo Since you've introduced this particular function, any objections to introduce a pointer to has_more flag, to make the function more generic?

[SeppoTakalo]

No objections.
I think the API has been formed a bit too much on what LwM2M client needed.

As it is still unstable, I would prefer someone to refactor it to be generic CoAP API that would be usable on both, client and server, side.

[boaks]

To fix the API is the on side.
The other side for me will still be to refer to the usage of the coap_block_context.