REST: Use If-Match header for incoming requests

Author: schlatterbeck

doc/rest.txt

@@ -133,6 +133,11 @@ inverse of ``retire``, the item is again visible.
 On success the returned value is the same as the respective ``GET``
 method.
 
+Note that the ``GET`` method on an item (e.g. ``/data/issue/43``)
+returns an ETag in the http header *and* the ``@etag`` value. When
+modifying the item via ``PUT`` or ``PATCH`` either a ``If-Match`` header
+or an ``@etag`` value in the form have to be provided.
+
 sample python client
 ====================
 
@@ -163,7 +168,7 @@ Retire/Restore::
         >>> print("ETag: %s" % etag)
         >>> etag = r.json()['data']['@etag']
         >>> print("@etag: %s" % etag)
-        >>> h = dict(ETag = etag)
+        >>> h = {'If-Match': etag}
         >>> d = {'@op:'action', '@action_name':'retire'}
         >>> r = s.patch(u + 'issue/42', data = d, headers = h)
         >>> print(r.json())

roundup/rest.py

@@ -135,7 +135,8 @@ def calculate_etag (node, classname="Missing", id="0"):
     etag = md5(bs2b(repr(sorted(items)))).hexdigest()
     logger.debug("object=%s%s; tag=%s; repr=%s", classname, id,
                  etag, repr(node.items(protected=True)))
-    return etag
+    # Quotes are part of ETag spec, normal headers don't have quotes
+    return '"%s"' % etag
 
 def check_etag (node, etags, classname="Missing", id="0"):
     '''Take a list of etags and compare to the etag for the given node.
@@ -151,7 +152,7 @@ def check_etag (node, etags, classname="Missing", id="0"):
     node_etag = calculate_etag(node, classname, id)
 
     for etag in etags:
-        if etag != None:
+        if etag is not None:
             if etag != node_etag:
                 return False
             have_etag_match=True
@@ -166,7 +167,7 @@ def obtain_etags(headers,input):
     etags = []
     if '@etag' in input:
         etags.append(input['@etag'].value);
-    etags.append(headers.get("ETag", None))
+    etags.append(headers.get("If-Match", None))
     return etags
 
 def parse_accept_header(accept):
@@ -459,6 +460,16 @@ def patch_data(self, op, old_val, new_val):
 
         return result
 
+    def raise_if_no_etag(self, class_name, item_id, input):
+        class_obj = self.db.getclass(class_name)
+        if not check_etag(class_obj.getnode(item_id),
+                obtain_etags(self.client.request.headers, input),
+                class_name,
+                item_id):
+            raise PreconditionFailed(
+                "If-Match is missing or does not match."
+                " Retrieve asset and retry modification if valid.")
+
     @Routing.route("/data/<:class_name>", 'GET')
     @_data_decorator
     def get_collection(self, class_name, input):
@@ -669,7 +680,7 @@ def get_element(self, class_name, item_id, input):
             '@etag': etag
         }
 
-        self.client.setHeader("ETag", '"%s"'%etag)
+        self.client.setHeader("ETag", etag)
         return 200, result
 
     @Routing.route("/data/<:class_name>/<:item_id>/<:attr_name>", 'GET')
@@ -717,7 +728,7 @@ def get_attribute(self, class_name, item_id, attr_name, input):
             '@etag': etag
         }
 
-        self.client.setHeader("ETag", '"%s"'%etag )
+        self.client.setHeader("ETag", etag)
         return 200, result
 
     @Routing.route("/data/<:class_name>", 'POST')
@@ -818,12 +829,7 @@ def put_element(self, class_name, item_id, input):
                     (p, class_name, item_id)
                 )
         try:
-            if not check_etag(class_obj.getnode(item_id),
-                       obtain_etags(self.client.request.headers, input),
-                       class_name,
-                       item_id):
-                raise PreconditionFailed("Etag is missing or does not match."
-                        " Retrieve asset and retry modification if valid.")
+            self.raise_if_no_etag(class_name, item_id, input)
             result = class_obj.set(item_id, **props)
             self.db.commit()
         except (TypeError, IndexError, ValueError) as message:
@@ -874,11 +880,7 @@ def put_attribute(self, class_name, item_id, attr_name, input):
         }
 
         try:
-            if not check_etag(class_obj.getnode(item_id),
-                        obtain_etags(self.client.request.headers, input),
-                        class_name, item_id):
-                raise PreconditionFailed("Etag is missing or does not match."
-                        " Retrieve asset and retry modification if valid.")
+            self.raise_if_no_etag(class_name, item_id, input)
             result = class_obj.set(item_id, **props)
             self.db.commit()
         except (TypeError, IndexError, ValueError) as message:
@@ -964,13 +966,7 @@ def delete_element(self, class_name, item_id, input):
                 'Permission to retire %s %s denied' % (class_name, item_id)
             )
 
-        if not check_etag(class_obj.getnode(item_id),
-                obtain_etags(self.client.request.headers, input),
-                class_name,
-                item_id):
-            raise PreconditionFailed("Etag is missing or does not match."
-                        " Retrieve asset and retry modification if valid.")
-
+        self.raise_if_no_etag(class_name, item_id, input)
         class_obj.retire (item_id)
         self.db.commit()
         result = {
@@ -1014,13 +1010,7 @@ def delete_attribute(self, class_name, item_id, attr_name, input):
             props[attr_name] = None
 
         try:
-            if not check_etag(class_obj.getnode(item_id),
-                       obtain_etags(self.client.request.headers, input),
-                       class_name,
-                       item_id):
-                raise PreconditionFailed("Etag is missing or does not match."
-                        " Retrieve asset and retry modification if valid.")
-
+            self.raise_if_no_etag(class_name, item_id, input)
             class_obj.set(item_id, **props)
             self.db.commit()
         except (TypeError, IndexError, ValueError) as message:
@@ -1064,12 +1054,7 @@ def patch_element(self, class_name, item_id, input):
             op = self.__default_patch_op
         class_obj = self.db.getclass(class_name)
 
-        if not check_etag(class_obj.getnode(item_id),
-                obtain_etags(self.client.request.headers, input),
-                class_name,
-                item_id):
-            raise PreconditionFailed("Etag is missing or does not match."
-                        " Retrieve asset and retry modification if valid.")
+        self.raise_if_no_etag(class_name, item_id, input)
 
         # if patch operation is action, call the action handler
         action_args = [class_name + item_id]
@@ -1173,12 +1158,7 @@ def patch_attribute(self, class_name, item_id, attr_name, input):
         prop = attr_name
         class_obj = self.db.getclass(class_name)
 
-        if not check_etag(class_obj.getnode(item_id),
-                obtain_etags(self.client.request.headers, input),
-                class_name,
-                item_id):
-            raise PreconditionFailed("Etag is missing or does not match."
-                        " Retrieve asset and retry modification if valid.")
+        self.raise_if_no_etag(class_name, item_id, input)
 
         props = {
             prop: self.prop_from_arg(

test/rest_common.py

@@ -375,7 +375,7 @@ def notestEtagGeneration(self):
     def testEtagProcessing(self):
         '''
         Etags can come from two places:
-           ETag http header
+           If-Match http header
            @etags value posted in the form
 
         Both will be checked if availble. If either one
@@ -401,7 +401,7 @@ def testEtagProcessing(self):
 
             if mode == 'header':
                 print("Mode = %s"%mode)
-                self.headers = {'etag': etag}
+                self.headers = {'if-match': etag}
             elif mode == 'etag':
                 print("Mode = %s"%mode)
                 form.list.append(cgi.MiniFieldStorage('@etag', etag))
@@ -411,11 +411,11 @@ def testEtagProcessing(self):
                 form.list.append(cgi.MiniFieldStorage('@etag', etag))
             elif mode == 'brokenheader':
                 print("Mode = %s"%mode)
-                self.headers = {'etag': 'bad'}
+                self.headers = {'if-match': 'bad'}
                 form.list.append(cgi.MiniFieldStorage('@etag', etag))
             elif mode == 'brokenetag':
                 print("Mode = %s"%mode)
-                self.headers = {'etag': etag}
+                self.headers = {'if-match': etag}
                 form.list.append(cgi.MiniFieldStorage('@etag', 'bad'))
             elif mode == 'none':
                 print( "Mode = %s"%mode)
@@ -451,7 +451,7 @@ def testDispatch(self):
         headers={"accept": "application/json",
                  "content-type": env['CONTENT_TYPE'],
                  "content-length": env['CONTENT_LENGTH'],
-                 "etag": etag
+                 "if-match": etag
         }
         self.headers=headers
         # we need to generate a FieldStorage the looks like
@@ -478,7 +478,8 @@ def testDispatch(self):
         # simulate: /rest/data/user/<id>/realname
         # use etag in payload
         etag = calculate_etag(self.db.user.getnode(self.joeid))
-        body=s2b('{ "@etag": "%s", "data": "Joe Doe 2" }'%etag)
+        etagb = etag.strip ('"')
+        body=s2b('{ "@etag": "\\"%s\\"", "data": "Joe Doe 2" }'%etagb)
         env = { "CONTENT_TYPE": "application/json",
                 "CONTENT_LENGTH": len(body),
                 "REQUEST_METHOD": "PUT",
@@ -492,7 +493,7 @@ def testDispatch(self):
 
         headers={"accept": "application/json",
                  "content-type": env['CONTENT_TYPE'],
-                 "etag": etag
+                 "if-match": etag
         }
         self.headers=headers # set for dispatch
 
@@ -516,7 +517,7 @@ def testDispatch(self):
         # Also use GET on the uri via the dispatch to retrieve
         # the results from the db.
         etag = calculate_etag(self.db.user.getnode(self.joeid))
-        headers={"etag": etag,
+        headers={"if-match": etag,
                  "accept": "application/json",
         }
         form = cgi.FieldStorage()
@@ -553,7 +554,8 @@ def testDispatch(self):
         self.assertEqual(self.dummy_client.response_code, 200)
 
         etag = calculate_etag(self.db.user.getnode(self.joeid))
-        body=s2b('{ "address": "[email protected]", "@etag": "%s"}'%etag)
+        etagb = etag.strip ('"')
+        body=s2b('{ "address": "[email protected]", "@etag": "\\"%s\\""}'%etagb)
         env = { "CONTENT_TYPE": "application/json",
                 "CONTENT_LENGTH": len(body),
                 "REQUEST_METHOD": "PATCH"
@@ -580,9 +582,10 @@ def testDispatch(self):
 
         # and set it back reusing env and headers from last test
         etag = calculate_etag(self.db.user.getnode(self.joeid))
-        body=s2b('{ "address": "%s", "@etag": "%s"}'%(
+        etagb = etag.strip ('"')
+        body=s2b('{ "address": "%s", "@etag": "\\"%s\\""}'%(
             stored_results['data']['attributes']['address'],
-            etag))
+            etagb))
         # reuse env and headers from prior test.
         body_file=BytesIO(body)  # FieldStorage needs a file
         form = client.BinaryFieldStorage(body_file,
@@ -665,7 +668,7 @@ def testPut(self):
             cgi.MiniFieldStorage('data', 'Joe Doe Doe'),
         ]
 
-        self.headers = {'etag': etag } # use etag in header
+        self.headers = {'if-match': etag } # use etag in header
         results = self.server.put_attribute(
             'user', self.joeid, 'realname', form
         )