search

icerpc / icerpc-csharp

A C# RPC framework built for QUIC, with bidirectional streaming, first-class async/await, and Protobuf support.
https://docs.icerpc.dev
Apache License 2.0
107 stars 13 forks source link

Should we Strictly Enforce the Decoding of `bool`s? #3752

Closed InsertCreativityHere closed 11 months ago

InsertCreativityHere commented 11 months ago

The encoding referece states that bools must be 0 for false or 1 for true, and that all other values are invalid.

However, the logic for DecodeBool doesn't enforce this part of the spec. It treats 0 as false, and any other value as true. Should we be better enforcing the spec and rejecting values like 2 as invalid, instead of silently treating them as valid?

For reference: https://github.com/icerpc/icerpc-csharp/blob/fbd611d007b8cadfc995e1474885262307decd1b/src/ZeroC.Slice/SliceDecoder.cs#L130-L131

Or alternatively, should we amend the spec to follow icerpc-csharp's behavior (0 is false, otherwise true). This would be a strict relaxing of the spec, and hence backwards compatible. It would simplify the job of a decoder (since it's impossible to decode an invalid bool with this change), but might be 'less safe'.

bernardnormier commented 11 months ago

The decoding must be strict. Receiving 5 for a bool is an error.

InsertCreativityHere commented 11 months ago

According to the C# docs, only the lowest bit of a bool is relevant in memory: https://learn.microsoft.com/en-us/dotnet/api/system.boolean?view=net-7.0#work-with-booleans-as-binary-values The other bits are free to be whatever, and C# doesn't care.

^ This is just in reference to a question. I agree that the decoder should be strict.