NatLabRockies / NatLabRockies/ninterp

Optionally validate on deserialization

Open
#17 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
17
Forks
13
Avg merge
44m
Merged PRs (30d)
2

Description

Depended on #36 (merged): splits Strategy::init into a pure validate and a
mutating init, and adds validate_strategy()/init_strategy() inherent methods to
each Interp*D. That split is what lets this issue call only the cheap check on
deserialize, without touching init at all, matching #36's own design intent of
decoupling the two rather than tethering them back together here.

Currently, deserializing an Interp*D struct populates fields directly without calling validate(). This means:

  • LinearUniform: a deserialized interpolator with a non-uniform grid will silently produce wrong results at query time
  • Step: a mismatched direction-count will pass validation silently

Proposed change

Use a proc macro to generate a *Wire helper struct for each interpolator. The real Deserialize impl uses #[serde(try_from = "Interp1DWire<D, S>")], and TryFrom calls data.validate() and validate_strategy(), both cheap, pure &self checks. On success, the interpolator is fully validated. On failure, deserialization returns an error instead of silently constructing broken state.

This does not call init_strategy(). That stays exactly what it already is today: a manual, opt-in method for anyone who needs guaranteed-fresh derived state, whether after deserializing a stateful custom strategy or after mutating data/strategy directly. Nothing about this issue changes that contract, on either the checked path or the deserialize_unvalidated escape hatch below. A strategy that serializes its own derived state (e.g. a future CubicSpline's precomputed coefficients) gets it back as-is, trusted, exactly the point of having serialized it in the first place; a strategy that skips a derived field via its own #[serde(skip, default)] already requires the caller to call init_strategy() manually, same as today.

This requires a proc-macro crate, which Rust requires to be separate from the crate using it ([lib] proc-macro = true can't coexist with regular library code). That means converting ninterp into a workspace with a new ninterp-macros crate, versioned in lockstep: the same split serde/serde_derive and thiserror/thiserror-impl use.

Escape hatch

Expose Interp1D::deserialize_unvalidated(raw: Interp1DRaw<D, S>) -> Self for callers who need to bypass validation (performance-critical hot paths, staged construction, etc.). The Interp1DRaw type is the plain derived struct: same wire format, no validation.

Not named deserialize_unchecked: that was considered and rejected on #18 for interpolate_fast, since _unchecked in Rust convention pairs with unsafe fn and implies UB on misuse (get_unchecked, from_utf8_unchecked). This is a logic-error contract, not a soundness one, the same reasoning as interpolate_fast over interpolate_unchecked. It's named for the thing it actually skips.

deserialize_unvalidated skips data.validate() and validate_strategy(). Since neither path ever touches init_strategy(), this is the only difference between the two: cheap invariant checks, on or off. This formalizes today's default behavior (deserialize populates fields, no validation) as an explicit, named opt-out, once validated deserialization becomes the default.

Behavior summary

Current Proposed
Invalid data from external source Silent wrong results Deserialization error
Valid round-trip (serialize -> deserialize) Works Works (validate passes trivially)
Stateful custom strategy needing fresh derived state Caller must call init_strategy() manually Unchanged: caller must still call init_strategy() manually
Bypass available N/A deserialize_unvalidated (skips data.validate()/validate_strategy() only)

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 by reading the Interp1D and Interp*D serialization paths, Strategy::init, validate_strategy(), and init_strategy(), then inspect the workspace and proc-macro crate requirements. Done means validated deserialization rejects invalid LinearUniform and Step state, valid round-trips still work, and deserialize_unvalidated bypasses only the cheap checks without calling init_strategy().

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
build-system, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.