scylladb / scylla-rust-driver

Async CQL driver for Rust, optimized for ScyllaDB!
Apache License 2.0
582 stars 101 forks source link

Accept out-of-range `time` values from DB and reject those supplied by the user #859

Open piodul opened 11 months ago

piodul commented 11 months ago

The CQL time value has the following specification:

6.16 time

  An 8 byte two's complement long representing nanoseconds since midnight.
  Valid values are in the range 0 to 86399999999999

Unfortunately, Scylla does not validate this range when a driver tries to insert a serialized time value.

Moreover, this driver does not validate it before sending either. On the other hand, we validate values received from Scylla and return an error during deserialization if it is out of range. Therefore, it is possible to use the driver to write a value which will cause an error when read back from the DB.

This is exactly the opposite to the robustness principle: we should not allow to send values that are out of range, and we should accept values that are out of range from DB.

Refs: https://github.com/scylladb/scylladb/issues/14667

mykaul commented 11 months ago

Is there a Scylla bug for this missing validation? @nyh - do you happen to know if we have relevant CQL tests for this?

piodul commented 11 months ago

Is there a Scylla bug for this missing validation?

Yes: https://github.com/scylladb/scylladb/issues/14667

wprzytula commented 6 months ago

This is exactly the opposite to the robustness principle: we should not allow to send values that are out of range, and we should accept values that are out of range from DB.

The robustness principle is phrased there as(in other words):

...programs that receive messages should accept non-conformant input as long as the meaning is clear.

I have no idea what could be the clear meaning of an out-of-range time value. Therefore, I believe that the driver should reject such values.

The Scylla side should definitely validate those values, so we're waiting for https://github.com/scylladb/scylladb/issues/14667 to be fixed.