diff --git a/app/controllers/1/syncs_controller.lua b/app/controllers/1/syncs_controller.lua index d866047..ee81dcd 100644 --- a/app/controllers/1/syncs_controller.lua +++ b/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 diff --git a/config/errors.lua b/config/errors.lua index 513839c..b3871f1 100644 --- a/config/errors.lua +++ b/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 diff --git a/spec/controllers/1/syncs_controller_spec.lua b/spec/controllers/1/syncs_controller_spec.lua index c06de1c..db5cfc2 100644 --- a/spec/controllers/1/syncs_controller_spec.lua +++ b/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)