bitemyapp / bitemyapp/esqueleto
(>=.) and friends have unsound types with respect to Maybe
- Dominant language
- Haskell
- Stars
- 399
- Forks
- 107
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 1
Description
If you use a comparison operator on a Maybe value, there is nothing stopping a null getting into your conditionals if the values going into `>=.` are actually Nothing. This may wind up ruining your day or otherwise creating ... funny runtime bugs, since you create a `NULL` in a `Value Bool`, which is not supposed to be there.
```haskell
oops :: SqlQuery (SqlExpr (Value Bool))
oops = do
pure (val Nothing >=. (val . Just) (2::Int))
```
I think that in the current type signatures, `val . Just` is probably universally an invitation for bugs with the *sole* exception of if the result goes into `||.`.
Real-world example
If you write a query like this and omit the `isNothing` check you can cause this issue:
```
-- | Schedule to accept calls
ScheduleEntry sql=schedule_entries
-- | Time at which this schedule starts applying
startDate Day Maybe
endDate Day Maybe
-- | Start time in each day that this schedule applies
startHours Int
startMins Int
endHours Int
endMins Int
```
```haskell
applicableRules :: LocalTime -> SqlQuery (SqlExpr (Entity ScheduleEntry))
applicableRules ts = do
sched <- from $ table @ScheduleEntry
let day = localDay ts
let TimeOfDay {todHour, todMin} = localTimeOfDay ts
where_ $ isNothing sched.startDate ||. (val . Just) day >=. sched.startDate
where_ $ isNothing sched.endDate ||. (val . Just) day <=. sched.endDate
where_ $ val todHour >=. sched.startHours &&. val todHour <=. sched.endHours
where_ $ val todMin >=. sched.startMins &&. val todMin <=. sched.endMins
pure sched
```
This could possibly be fixed in a way that only breaks suspicious uses (although my condolences extend to anyone with a large codebase dealing with migrating this; probably would want to keep around the old buggy signatures for compat under a different name, so you could just replace all usages and migrate piecewise):
```haskell
type family BoolOpResult (a :: Type) :: Type where
BoolOpResult (Maybe a) = Maybe Bool
BoolOpResult a = Bool
(>=.) :: (PersistField typ) => SqlExpr (Value typ) -> SqlExpr (Value typ) -> SqlExpr (Value (BoolOpResult typ))
(>=.) = unsafeSqlBinOp ">= "
```
cc @parsonsmatt, I'm unsure if this type level shenanigans is fully sound either
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.