clockworklabs / clockworklabs/SpacetimeDB

`bsatn::from_slice` should return an error if it does not consume the whole slice

Open
#1,942 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
25.2k
Forks
1.1k
Avg merge
2d 7h
Merged PRs (30d)
46

Description

There seems to be no bsatn validation

  • Add field to existing reducer type
  • Generate client code
  • Forget to publish updated server
  • Reducer still runs, but data is deserialized incorrectly
    I know proper versioning is difficult, but a check like "if we're done deserializing everything and there's still data in stream then throw error" would be nice

Unlike bsatn::from_reader, from_slice is only ever useful to read a single instance from a slice, as it does not return or signal where a caller would continue reading the next element. As such, from_slice should return an error if it does not read the slice to the end, as this most likely represents a type error. bsatn::from_reader should not change, as it remains useful to use that function to read multiple elements sequentially out of a single reader.

  • Extend sats::buffer::DecodeError with a variant which represents this error case.
  • Alter sats::bsatn::from_slice to hold onto its slice-reader in a named variable rather than an implicit temporary, and to check that the slice-reader is empty after its call to from_reader. If the slice-reader is not empty, it should return the above error.
  • Add a unit test in bsatn.rs which demonstrates this error return by from_slice.
  • Stretch goal: add an integration test, in the smoketests and/or SDK tests, which demonstrates that WebSocket calls using a too-short BSATN arguments product to a reducer is detected as a serialization error. Only calls via WebSockets are meaningful, as the HTTP API uses JSON serialized reducer arguments.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with sats::buffer::DecodeError and sats::bsatn::from_slice, then inspect the existing unit tests in bsatn.rs. Run the BSATN tests and add coverage for a slice containing trailing data after one decoded value. Done means from_slice reports the new error while from_reader still supports sequential reads.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.