Monday, February 26, 2018

Optimistic Concurrency Control (22)

Last post, I put in code to stop incorrect changes being made by malicious actors.

This post, I want to put in code to stop incorrect changes being made by innocent actors.

An example is the case that the form is submitted twice "at the same time", perhaps because it was opened on two different computers, submitted on the first, then later submitted on the second (or different browser tabs on the same computer). (I hit this bug with reddit quite often, and my bug submission there is one of the most disappointing bug responses I've ever encountered).

Because the was was opened the second time before the first instance was submitted, the user interface presents it as editable and waiting for submission, and continues to do so after the first instance has been submitted - for as long as you don't reload that page on the second computer.

A pretty common technique to deal with this is called optimistic concurrency control (OCC).

OCC assigns a version identifier to each version of the data. When a change is submitted, it contains the version against which the change is being made. If that version identifier isn't the same as the version in the database, the change is aborted.

If you're used to version control systems like git, you can imagine this as a very lightweight version of git's commit identifiers. Instead of storing the whole commit history, only the latest head is stored; and many of the interesting things git can do, like reconcile multiple changes against a history commit by merging can't happen in this system. (if you don't know about git, ignore this paragraph)

If you're used to transactions - this is a transaction system which can only enforce consistency against a single versioned record: you can't do SQL-style transactional changes to multiple records (unless they all have one single shared version identifier - which increases the likelihood of conflict).

It doesn't matter too much what database type is used for version identifiers, as long as you can generate new ones, and compare them.

I'm going to use a timestamp. This is quite a complicated type, but allows the field to double as a possibly-useful last-modified time.

First use the database migration system to update the registration table so that each record has a version. It doesn't matter what we use as an initial value for existing records; I'll use the current time.

In migrations/0006-occ.sql:

ALTER TABLE registration ADD COLUMN occ TIMESTAMP WITH TIME ZONE;
UPDATE registration SET occ = NOW();
ALTER TABLE registration ALTER COLUMN occ SET NOT NULL;

Without any further changes, we now can't create any new registrations although we can modify existing ones.

The Haskell-side Registration type now needs to know about this.

There's a postgresql-simple type for storing timestamps, which we can use in the definition of Registration.

import qualified Database.PostgreSQL.Simple.Time as PGT
...
data Registration = Registration {
...
    occ :: PGT.ZonedTimestamp,
...
}

There's immediately a build error:

    * No instance for (CSV.ToField
                         (postgresql-simple-0.5.3.0:Database.PostgreSQL.Simple.Time.Implementation.Unbounded
                            time-1.8.0.2:Data.Time.LocalTime.Internal.ZonedTime.ZonedTime))

... which is complaining that cassava doesn't know how to write a Registration out to CSV format any more - specifically because I doesn't know how to write out postgresql-simple timestamps.

Again I'm going to avoid wrapping this into its own type and specifying instances there. Instead I'm going to add a definition to our shifty Orphans.hs file:

{-# Language FlexibleInstances #-}
{-# Language TypeSynonymInstances #-}
...
import Database.PostgreSQL.Simple.Time as PGT
...
instance CSV.ToField PGT.ZonedTimestamp
  where
    toField time = CSV.toField (show time)

Those Language pragmas are needed because ZonedTimestamp is actually an alias for a more complicated type that can't by default have typeclass instances declared on it. This instance will write the timestamp to CSV as a string.

In Main.hs, the only place we create a Registration from scratch is in doInvitation. When constructing that registration we'll need an initial database timestamp. One lazy way to do that is go ask the database for one. (Another way would be to ask the Haskell runtime for the current time, and convert it into a ZonedTimestamp)

In src/DB.hs, the place for database library functions, add this:

import qualified Database.PostgreSQL.Simple.Time as PGT
...
generateOCC :: IO PGT.ZonedTimestamp
generateOCC = do
  [[n]] <- withDB $ \conn -> PG.query conn "SELECT NOW()" ()
  return n

... which will ask the database what time is NOW().

Now in doInvite we can add:

...
  newOCC <- generateOCC
...

     nonce = Just newNonce,
     occ = newOCC,
     email = Just (I.email invitation),
...

In registrationDigestiveForm we can add this OCC field initially as a constant value that cannot be modified:

registrationDigestiveForm initial = do
  Registration
...
    <*> (pure . occ) initial
...

What we should have now is a registration which generates an initial OCC version identifier when a registration is initially added, but doesn't ever change it or act on it. Nevertheless the code should compile and run at this point.

Now I want to actually use this new field to do some concurrency control.

Rather than this field being a constant loaded from the database at each iteration of form processing (so always having the latest value), instead I want to send it to the user's browser as part of the form so that when a form submission comes back, I know which version of the form data they were modifying.

digestive-functors provides DF.stringRead to define a form field that can be converted to/from string representation using the standard Show and Read typeclasses.

In registrationDigestiveForm, get rid of the pure . occ implementation of the OCC field, and instead use

...
    <*> "occ" .: DF.stringRead "OCC Version" (Just $ occ initial)
...

This new string field can be invisibly sent as part of the HTML form, to be returned as a POST parameter using inputHidden from digestive-functors-blaze.

Add this:

      inputHidden "occ" view

... anywhere in the HTML form definition in htmlForRegistration.

If you build and compile that, and look at the HTML page source of a registration form, you should see something like this:

<input type="hidden" id="Registration.occ" name="Registration.occ" value="2018-02-25 18:15:44.392519 +0000">

... which will be sent back on each submission.

... but this is still not doing any checking...

I'm going to check in two places: in the user interface where nice errors can be presented to the user; and nearer the database where I can be a bit surer that there are not race conditions.

I'll implement the database level checking first.

At present, updates happen with a call like this:

  withDB $ \conn -> gupdateInto conn "registration" "nonce = ?" n [identifier]

which returns the number of rows that have been updated, and then ignores that number.

I'm going to change the WHERE clause of that update: instead of only selecting rows with the appropriate nonce, I'm going to further narrow down to rows with the correct OCC version; and when writing out the new row update the OCC (in the same way that the status is updated); and then check that exactly one row was updated.

import Control.Monad (when)
...
    (_, Just newRegistration) -> do
      let oldOcc = occ newRegistration
      newOcc <- liftIO $ generateOCC
      let n = newRegistration { status = "C", occ = newOcc}
      rows <- withDB $ \conn -> gupdateInto conn "registration" "nonce = ? AND occ = ?" n (identifier, occ newRegistration)
      when (rows /= 1) $ error "Registration POST did not update exactly one row"
      return "Record updated."

That rows /= 1 is quite rough: there are other reasons why it might not work, such as the registration being duplicated, or not in the database at all any more. But it will do for now.

With this in place, open the same registration form in two browser tabs, and try to submit changes from first one, then the other. The first submission should work, and the second should fail.

So we get an ugly error message.

Another place we can check this is in form validation. This has a race condition, though, which is why the above database check is necessary: two submissions of the same form could both be validated, before the two submissions hit the database. That's quite unlikely though so I haven't put much effort into the error message above.

More likely that race condition won't happen and we will be fine with form-level validation.

In registrationDigestiveForm, put the field definition for "occ" in its own function:

    <*> "occ" .: occVersion (occ initial)

... which will have the same stringRead definition as before, but with a validator on top.

occVersion dbOCC =   
  DF.check  
    "This form has been modified by someone else - please reload"
    (\newOCC -> show newOCC == show dbOCC)
  $ DF.stringRead "OCC Version" (Just $ dbOCC)

That will check that the most recent version of the data loaded from the database at POST time matches the version sent in the POST request.

This will now cause the form to refuse to be submitted, telling the user that there were errors.

But it doesn't tell the user where that error occurred - we need to put DB.errorList "occ" view in htmlForRegistration at the point where we want OCC errors to be reported. With individual editable fields, it makes sense to put that error list for each field's input; but there isn't a relevant visible input field for occ. I've put the error list near the top of the HTML.

So now most of the time, forms will submit just as before. If there has been a simultaneous edit, then most of the time, the error from digestive-functors will be sent back; but (hopefully only very occasionally) there will be a database level error with a not-very-helpful error returned instead.

Eliminating that last case while still using digestive-functors can probably be done, but involves a bit more interplay between explicit SQL transactions and validation: perhaps, run the whole form validation and update inside an SQL transaction and if it aborts due to a conflict, run it again, and again, and again, hoping that the second time round form validation will fail.

What about if the user is malicious and fiddles their occ value? All they are able to do is update data that they could already have updated by loading the form fresh and fiddling in there. This is a protection against user mistakes, not against users being able to change data.

Here's the link to this post's commit.

No comments:

Post a Comment