Bug in URI Validator forcing to set service_instance_id to be 1
Closed this issue · 4 comments
It is perfectly valid if the ue_id (32bits) has the upper 16 bits (the instance_id portion) set to 0 meaning we are not caring about the instance id however isValidRpcMethod(), isValidRpcResponse(), isValidPublishTopic() all will fail if the upper 16 bits are 0 because uses_wildcards() returns true when the upper 16 bits is 0.
Examples of the issue can be found in https://github.com/eclipse-uprotocol/up-cpp/blob/main/test/coverage/datamodel/UUriValidatorTest.cpp#L43.
I confirmed that the specifications is ambiguous to this point so I will also add an issue to up-spec to fix this, eclipse-uprotocol/up-spec#217
There has been some fairly extensive discussion on this. There are a few issues here.
First, overloading the value of 0
to be both a valid instance ID and the wildcard filter for instance IDs creates a scenario where developers could easily and unintentionally create invalid configurations (such as having two instances with ID 0
and 1
- instance 0
would receive all messages for all instances and instance 1
would receive only its messages). There's no clear way for us to enforce a rule that if 0
is used as an instance ID, other values should not be used (for a given entity ID).
Second, it was mentioned that an instance ID of 0
cannot be used directly in the SOME/IP transport format, and so an instance ID of 0
is always rewritten to be 1
in SOME/IP. This creates an ambiguous mapping since both instance ID 0
and instance ID 1
could be represented as instance ID 1
over SOME/IP. This once again invites misconfiguration and unclear behavior.
Third, we want to preserve instance ID 0
as a valid instance ID since protobuf will pack integers into smaller fields to avoid sending unnecessary 0
bytes. Many entities may have only one instance, so in aggregate this one byte per message savings is non-trivial. This means we can't solve the above issues by stating that valid instance IDs start at 1
and reserve 0
to be the wildcard filter value.
Given those issues, my recommendation is to update the spec as follows:
-
Make the instance ID wildcard value
0xFFFF
. Since the filters will rarely be sent on the wire, this preserves0
as a normal instance ID. This also has benefits when doing pattern matching with things like bloom filters, and is consistent with the pattern used in all other numeric UUri fields. -
Disallow
0xFFFF
as an instance ID value in message attributes (just like all the other URI field wildcards). -
For uProtocol to SOME/IP mapping, always add
1
to the instance ID. For SOME/IP to uProtocol mapping, always subtract1
from the instance ID.0xFFFF
would be allowed in SOME/IP messages since it represents a uProtocol instance ID of0xFFFE
Labeling this bug as blocked until agreement is reached on whether a spec update is needed.
The spec is being updated. We will need to update up-cpp's handling of instance ID to treat 0xFFFF
as the wildcard value now. See eclipse-uprotocol/up-spec#222
This fix should target the v1.0_up-v1.6.0
branch