Refuse document ids the read route cannot serve

app/controllers/1/syncs_controller.lua

@@ -18,6 +18,7 @@ local SyncsController = {
     error_document_field_missing = 2004,
     error_user_registration_disabled = 2005,
     error_account_not_found = 2006,
+    error_document_not_servable = 2007,
 }
 
 local null = ngx.null
@@ -81,6 +82,14 @@ local function is_valid_key_field(field)
     return is_valid_field(field) and not string.find(field, ":")
 end
 
+-- The read route binds :document against a fixed character class (see
+-- gin/core/routes.lua, build_named_parameters), so an id outside it is stored
+-- by PUT and then 404s at nginx before the controller is reached. Refuse it on
+-- the way in rather than accepting a write that can never be read back.
+local function is_servable_document(field)
+    return string.match(field, "^[A-Za-z0-9_]+$") ~= nil
+end
+
 function SyncsController:getRedis()
     local redis = Redis:new()
     if not redis then
@@ -238,6 +247,9 @@ function SyncsController:update_progress()
     if not is_valid_key_field(doc) then
         self:raise_error(self.error_document_field_missing)
     end
+    if not is_servable_document(doc) then
+        self:raise_error(self.error_document_not_servable)
+    end
 
     local percentage = tonumber(self.request.body.percentage)
     local progress = self.request.body.progress

config/errors.lua

@@ -21,6 +21,7 @@ local Errors = {
     [2004] = { status = 403, message = "Field 'document' not provided.", },
     [2005] = { status = 402, message = "User registration is disabled.", },
     [2006] = { status = 404, message = "Account not found.", },
+    [2007] = { status = 403, message = "Field 'document' contains characters the read route cannot serve.", },
 }
 
 return Errors

spec/controllers/1/syncs_controller_spec.lua

@@ -440,5 +440,24 @@ describe("SyncsController", function()
                 device = "my pb"
             }, response.body)
         end)
+
+        it("refuses a document id the read route cannot serve", function()
+            for _, bad in ipairs({ "has-a-hyphen", "has.a.dot", "has a space" }) do
+                local response = update(username, userkey, bad, 0.5, "10", "kpw")
+                assert.are.same(403, response.status)
+                assert.are.same(2007, response.body.code)
+                -- and the read route still cannot serve it, which is the point.
+                -- nginx answers 404 for a hyphen or a dot and 400 for a space,
+                -- so assert only that it is never served.
+                assert.is_true(get(username, userkey, bad).status ~= 200)
+            end
+        end)
+
+        it("still accepts the ids real clients send", function()
+            for _, ok in ipairs({ "0b229176d4e8db7f6d2b5a4952368d7a", "under_score", "MiXeD123" }) do
+                local response = update(username, userkey, ok, 0.5, "10", "kpw")
+                assert.are.same(200, response.status)
+            end
+        end)
     end)
 end)