Merge lp:~pedronis/u1db/naming-constraints into lp:u1db

[Samuele Pedroni]

* start imposing constraints on:

db names (for remote access): [a-zA-Z0-9][a-zA-Z0-9.-]*

doc ids: [^\\/]+

* fix quoting issues when constructing URLs in HTTPClient, e.g. %FFFF should be a supported doc id

[John A Meinel (jameinel)]

I think:

ERROR_STATUSES.add(400)

is clearer than doing the set "|" operations.

Otherwise, looks good.

review: Approve

[lp:~pedronis/u1db/naming-constraints]

137. By Samuele Pedroni on 2011-12-01

style tweak

=== modified file 'u1db/__init__.py'
--- u1db/__init__.py	2011-11-30 13:14:11 +0000
+++ u1db/__init__.py	2011-12-01 15:18:24 +0000
@@ -33,6 +33,13 @@
     return sqlite_backend.SQLiteDatabase.open_database(path, create=create)
 
 
+# constraints on database names (relevant for remote access, as regex)
+DBNAME_CONSTRAINTS = r"[a-zA-Z0-9][a-zA-Z0-9.-]*"
+
+# constraints on doc ids (as regex)
+DOC_ID_CONSTRAINTS = r"[^/\\]+"
+
+
 class Database(object):
     """A JSON Document data store.
 

=== modified file 'u1db/backends/__init__.py'
--- u1db/backends/__init__.py	2011-11-28 15:16:41 +0000
+++ u1db/backends/__init__.py	2011-12-01 15:18:24 +0000
@@ -14,11 +14,19 @@
 
 """"""
 
+import re
+
 import u1db
+from u1db import (
+    errors,
+)
 import u1db.sync
 from u1db.vectorclock import VectorClockRev
 
 
+check_doc_id_re = re.compile("^" + u1db.DOC_ID_CONSTRAINTS + "$")
+
+
 class CommonSyncTarget(u1db.sync.LocalSyncTarget):
     pass
 
@@ -34,6 +42,10 @@
         vcr.increment(self._replica_uid)
         return vcr.as_str()
 
+    def _check_doc_id(self, doc_id):
+        if not check_doc_id_re.match(doc_id):
+            raise errors.InvalidDocId()
+
     def _get_generation(self):
         raise NotImplementedError(self._get_generation)
 

=== modified file 'u1db/backends/inmemory.py'
--- u1db/backends/inmemory.py	2011-11-29 14:45:48 +0000
+++ u1db/backends/inmemory.py	2011-12-01 15:18:24 +0000
@@ -61,6 +61,7 @@
     def put_doc(self, doc):
         if doc.doc_id is None:
             raise errors.InvalidDocId()
+        self._check_doc_id(doc.doc_id)
         if self._has_conflicts(doc.doc_id):
             raise errors.ConflictedDoc()
         old_doc = self._get_doc(doc.doc_id)

=== modified file 'u1db/backends/sqlite_backend.py'
--- u1db/backends/sqlite_backend.py	2011-12-01 14:53:45 +0000
+++ u1db/backends/sqlite_backend.py	2011-12-01 15:18:24 +0000
@@ -269,6 +269,7 @@
     def put_doc(self, doc):
         if doc.doc_id is None:
             raise errors.InvalidDocId()
+        self._check_doc_id(doc.doc_id)
         with self._db_handle:
             if self._has_conflicts(doc.doc_id):
                 raise errors.ConflictedDoc()

=== modified file 'u1db/errors.py'
--- u1db/errors.py	2011-11-28 19:49:13 +0000
+++ u1db/errors.py	2011-12-01 15:18:24 +0000
@@ -34,6 +34,7 @@
 class InvalidDocId(U1DBError):
     """A document was tried with an invalid document identifier."""
 
+    wire_description = "invalid document id"
 
 class ConflictedDoc(U1DBError):
     """The document is conflicted, you must call resolve before put()"""

=== modified file 'u1db/remote/http_app.py'
--- u1db/remote/http_app.py	2011-11-30 14:25:18 +0000
+++ u1db/remote/http_app.py	2011-12-01 15:18:24 +0000
@@ -27,6 +27,7 @@
     __version__ as _u1db_version,
     Document,
     errors,
+    DBNAME_CONSTRAINTS,
     )
 from u1db.remote import (
     http_errors,
@@ -149,7 +150,8 @@
     def register(self, resource_cls):
         # register
         self._map.connect(None, resource_cls.url_pattern,
-                          resource_cls=resource_cls)
+                          resource_cls=resource_cls,
+                          requirements={"dbname": DBNAME_CONSTRAINTS})
         self._map.create_regs()
         return resource_cls
 

=== modified file 'u1db/remote/http_client.py'
--- u1db/remote/http_client.py	2011-11-29 10:26:37 +0000
+++ u1db/remote/http_client.py	2011-12-01 15:18:24 +0000
@@ -69,7 +69,12 @@
     def _request(self, method, url_parts, params=None, body=None,
                                                        content_type=None):
         self._ensure_connection()
-        url_query = '/'.join([self._url.path] + url_parts)
+        url_query = self._url.path
+        if url_parts:
+            if not url_query.endswith('/'):
+                url_query += '/'
+            url_query += '/'.join(urllib.quote(part, safe='')
+                                  for part in url_parts)
         if params:
             url_query += ('?' +
                       urllib.urlencode(dict((unicode(v).encode('utf-8'),

=== modified file 'u1db/remote/http_errors.py'
--- u1db/remote/http_errors.py	2011-11-29 17:12:28 +0000
+++ u1db/remote/http_errors.py	2011-12-01 15:18:24 +0000
@@ -21,6 +21,7 @@
 
 # error wire descriptions mapping to HTTP status codes
 wire_description_to_status = dict([
+    (errors.InvalidDocId.wire_description, 400),
     (errors.DatabaseDoesNotExist.wire_description, 404),
     (errors.DocumentDoesNotExist.wire_description, 404),
     (errors.DocumentAlreadyDeleted.wire_description, 404),
@@ -30,6 +31,7 @@
 ])
 
 
-# 400 included for tests/anticipated usage
-ERROR_STATUSES = set(wire_description_to_status.values())|set([400])
 
+ERROR_STATUSES = set(wire_description_to_status.values())
+# 400 included explicitly for tests
+ERROR_STATUSES.add(400)

=== modified file 'u1db/tests/test_backends.py'
--- u1db/tests/test_backends.py	2011-11-29 14:45:48 +0000
+++ u1db/tests/test_backends.py	2011-12-01 15:18:24 +0000
@@ -89,6 +89,12 @@
         doc = Document(None, None, simple_doc)
         self.assertRaises(errors.InvalidDocId, self.db.put_doc, doc)
 
+    def test_put_doc_refuses_slashes(self):
+        doc = Document('/a', None, simple_doc)
+        self.assertRaises(errors.InvalidDocId, self.db.put_doc, doc)
+        doc = Document(r'\b', None, simple_doc)
+        self.assertRaises(errors.InvalidDocId, self.db.put_doc, doc)
+
     def test_put_doc_refuses_non_existing_old_rev(self):
         doc = Document('doc-id', 'test:4', simple_doc)
         self.assertRaises(errors.RevisionConflict, self.db.put_doc, doc)

=== modified file 'u1db/tests/test_http_app.py'
--- u1db/tests/test_http_app.py	2011-11-30 13:46:44 +0000
+++ u1db/tests/test_http_app.py	2011-12-01 15:18:24 +0000
@@ -439,6 +439,32 @@
         self.assertEqual('application/json', resp.header('content-type'))
         self.assertEqual({}, simplejson.loads(resp.body))
 
+    def test_valid_database_names(self):
+        resp = self.app.get('/a-database', expect_errors=True)
+        self.assertEqual(404, resp.status)
+
+        resp = self.app.get('/db1', expect_errors=True)
+        self.assertEqual(404, resp.status)
+
+        resp = self.app.get('/0', expect_errors=True)
+        self.assertEqual(404, resp.status)
+
+        resp = self.app.get('/0-0', expect_errors=True)
+        self.assertEqual(404, resp.status)
+
+        resp = self.app.get('/org.future', expect_errors=True)
+        self.assertEqual(404, resp.status)
+
+    def test_invalid_database_names(self):
+        resp = self.app.get('/.a', expect_errors=True)
+        self.assertEqual(400, resp.status)
+
+        resp = self.app.get('/-a', expect_errors=True)
+        self.assertEqual(400, resp.status)
+
+        resp = self.app.get('/_a', expect_errors=True)
+        self.assertEqual(400, resp.status)
+
     def test_put_doc_create(self):
         resp = self.app.put('/db0/doc/doc1', params='{"x": 1}',
                             headers={'content-type': 'application/json'})

=== modified file 'u1db/tests/test_http_client.py'
--- u1db/tests/test_http_client.py	2011-11-28 17:08:19 +0000
+++ u1db/tests/test_http_client.py	2011-12-01 15:18:24 +0000
@@ -111,6 +111,11 @@
                           'QUERY_STRING': 'a=1',
                           'REQUEST_METHOD': 'GET'}, simplejson.loads(res))
 
+        res, headers = cli._request('GET', ['doc', '%FFFF', 'echo'], {'a': 1})
+        self.assertEqual({'PATH_INFO': '/dbase/doc/%FFFF/echo',
+                          'QUERY_STRING': 'a=1',
+                          'REQUEST_METHOD': 'GET'}, simplejson.loads(res))
+
         res, headers = cli._request('POST', ['echo'], {'b': 2}, 'Body',
                                    'application/x-test')
         self.assertEqual({'CONTENT_TYPE': 'application/x-test',

=== modified file 'u1db/tests/test_http_database.py'
--- u1db/tests/test_http_database.py	2011-11-30 13:18:42 +0000
+++ u1db/tests/test_http_database.py	2011-12-01 15:18:24 +0000
@@ -181,3 +181,12 @@
         db = http_database.HTTPDatabase.open_database(self.getURL('new'),
                                                       create=True)
         self.assertIs(None, db.get_doc('doc1'))
+
+    def test_doc_ids_needing_quoting(self):
+        db0 = self.request_state._create_database('db0')
+        db = http_database.HTTPDatabase.open_database(self.getURL('db0'),
+                                                      create=False)
+        doc = Document('%fff', None, '{}')
+        db.put_doc(doc)
+        self.assertGetDoc(db0, '%fff', doc.rev, '{}', False)
+        self.assertGetDoc(db, '%fff', doc.rev, '{}', False)