Handle untrusted input

[mgsloan]

I think that the Store class and Peek / Poke / Size should all have an added type parameter or two which customizes the details of serialization. In particular, it should specify whether the input is trusted or untrusted, and whether architecture independent serialization should be used https://github.com/fpco/store/issues/36. Let's just focus on Trusted vs Untrusted for now.

One tricky thing is handling boxed Vector (). This is tricky because the size of () is 0 when serialized, but when deserialized it takes up space. Haven't tried compiling this, but I think something roughly like the following could work alright (after enabling a zillion extensions):

import GHC.Exts
import GHC.TypeLits
import Data.Type.Bool

data Trusted = InputTrusted | InputUntrusted

type family IsUntrusted (t :: Trusted) :: Bool where
  IsUntrusted InputTrusted = 'False
  IsUntrusted InputUntrusted = 'True

class Store (t :: Trusted) a where
  type IsUnitLike a :: Bool
  -- NOTE: Size / Poke don't need the trusted parameter
  size :: Size a
  poke :: a -> Poke a
  peek :: Peek t a

-- | Disallow unit-like types (zero size) when input is untrusted.
type UnitIsUntrusted (t) (a) :: Constraint =
  If (IsUntrusted t && Not (IsUnitLike a))
     (TypeError (Text "Input is untrusted, but the type " :<>: ShowType a :<> Text " may be size 0 and is used in an unsafe context.")
     ()

instance (UnitIsUntrusted t a, Store a) => Store t (Vector a) where

Main issue is that this is some seriously fancy type level stuff. Not 100% sure if the custom type error would work.

For some other things, like having extra checks for Map invariants, this would be handled by reifying Trusted as a value and using a conditional.

[jm4games]

Maybe I missed this, but what does the trusted/untrusted get you. Are the wins great enough that it justifies breaking everything?

[mgsloan]

@jm4games The reason for this is that I am potentially reverting some partial safety measures in https://github.com/fpco/store/pull/123 . The problem is that there is a tradeoff between performance and dealing with untrusted input gracefully. Not sure, but I think that full support for untrusted deserialization will inolve making Peek a bit more complicated to pass around state. The degree of inlining / optimization is fairly sensitive to this, so it'd be best to only pass around this extra state when necessary.

We could actually minimize breakage by having a different name for the class and exporting a constraint synonym type Store = Store' Untrusted a. Then it would only break users that have manual store instances. I suppose they might need to enable ConstraintKinds as well. Inclined to prefer a breaking change so that it encourages people to consider whether or not they trust the input.

[jm4games]

Ok, I will have to take a look at 123. This only peeked my interest since I happen to have a lot of manual store instances :(

[mgsloan]

As proposed here, the migration would only require you to add a bit of boilerplate per instance

instance Store Thing where
  ...

would become

instance Store t Thing where
  type IsUnitLike Thing = False

An alternative approach to consider for () would actually be to allow it, but force it to be encoded as a single byte when the input is untrusted. This'd be ugly though, because then the trusted and untrusted serialization would differ.

[mgsloan]

I ended up having a different reason for wanting a type parameter on every Store / Size / Peek / Poke, but it could be useful for this in the future - https://github.com/mgsloan/store/commit/00d77d5ec0f4e8be6a4f94699b7abbca1addb694

For now this change is experimental. It breaks the whole API, so I imagine that a polished up version of this that could be merged to this repo would attempt to minimize such breakage.