eclipse-uprotocol / up-spec

uProtocol Specifications
Apache License 2.0
32 stars 25 forks source link

Ambiguous definition of Micro URI with ID Remote #99

Closed gregmedd closed 4 months ago

gregmedd commented 5 months ago

In the URI Spec, the micro encoding of a Remote ID URI is shown as containing two fields: an 8-bit ID_LEN followed by a variable length UAUTHORITY_ID. An annotation is provided on the UAUTHORITY_ID reading (1=256 bytes).

This (1=256 bytes) implies that the allowable size range for the ID is from 1 to 256 inclusive. Since the ID_LEN is 8-bits, it follows that its expected to store id.size() - 1 such that an ID with a single byte would be represented by a ID_LEN of 0. This is necessary if we read the allowable size range as inclusive of 256 as that value cannot be represented in an 8-bit unsigned integer.

However, Table 3 states that the requirements for ID_LEN are that it "MUST be greater than 0", implying that the actual range is 1 to 255 inclusive, with an ID_LEN value of 0 being an invalid value. That same table defines UAUTHORITY_ID as being "8-2040 bits" (2040 bits == 255 bytes), supporting this interpretation.

To remove the ambiguity, either the diagram in 4.2.2 should be updated to more clearly state the actual size range of UAUTHORITY_ID, or Table 3 should be updated to allow for up to 2048 bits in the ID while removing the language that a length of zero is disallowed.

PLeVasseur commented 5 months ago

The (1=256 bytes) is what's throwing you from this table, right?

 0                   1                   2                   3
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|  UP_VERSION   |      TYPE     |           URESOURCE_ID        |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|        UENTITY_ID             |  UE_VERSION   |   UNUSED      |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|    ID_LEN     |        UAUTHORITY_ID (1=256 bytes)  ...       |
|                                                               |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

I agree it conflicts with the other pieces of information in that section of the spec. My guess is that the UAUTHORITY_ID is intended to be [1, 255] in length.

In up-rust we comply with the spec: image by having a set of validate_micro_form() functions for UAuthority, UEntity, UResource.

In particular the one for UAuthority checks the size is [1, 255].

gregmedd commented 5 months ago

Yes, that's the bit that is causing the conflict. I've been working to those diagrams, and from that standpoint it makes sense to use the full range of ID_LEN with the max UAUTHORITY_ID having 256 bytes. It's never valid to have zero bytes in the ID, so there isn't a reason to have a micro encoding that can represent an empty ID (if that were to change, we would presumably bump UP_VERSION).

However, if up-rust has already implemented this as [1, 255], then up-cpp can follow that. The spec should probably be updated so that table doesn't conflict with the earlier text.

stevenhartley commented 5 months ago

Hi @gregmedd you are right the limit is 1-255, can you push a fix to update the document (to correct this)? Thank you!!

stevenhartley commented 4 months ago

We are phasing out micro URI so this issue will be closed