From 0f3b35d4e8a2e3711c84f4c608cfc15b0c11b0f5 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Sun, 30 Dec 2018 21:00:54 -0800
Subject: [PATCH] Experimental new table / encoding schema

See https://github.com/django/asgiref/issues/51#issuecomment-450603464
---
 datasette/app.py                  |  2 ++
 datasette/templates/database.html |  2 +-
 datasette/templates/index.html    |  2 +-
 datasette/templates/row.html      |  4 ++--
 datasette/templates/table.html    |  2 +-
 datasette/utils.py                | 17 +++++++++++++++++
 datasette/views/base.py           | 12 +++++++-----
 datasette/views/table.py          |  5 +++--
 tests/test_api.py                 |  4 ++--
 tests/test_html.py                |  8 ++++----
 tests/test_utils.py               | 13 +++++++++++++
 11 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/datasette/app.py b/datasette/app.py
index cdfcb11d..c73c0fe0 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -27,6 +27,7 @@ from .views.table import RowView, TableView
 from .utils import (
     InterruptedError,
     Results,
+    encode_table_name,
     escape_css_string,
     escape_sqlite,
     get_plugins,
@@ -465,6 +466,7 @@ class Datasette:
         self.jinja_env = Environment(loader=template_loader, autoescape=True)
         self.jinja_env.filters["escape_css_string"] = escape_css_string
         self.jinja_env.filters["quote_plus"] = lambda u: urllib.parse.quote_plus(u)
+        self.jinja_env.filters["encode_table_name"] = encode_table_name
         self.jinja_env.filters["escape_sqlite"] = escape_sqlite
         self.jinja_env.filters["to_css_class"] = to_css_class
         pm.hook.prepare_jinja2_environment(env=self.jinja_env)
diff --git a/datasette/templates/database.html b/datasette/templates/database.html
index 05acff84..9c581656 100644
--- a/datasette/templates/database.html
+++ b/datasette/templates/database.html
@@ -27,7 +27,7 @@
 {% for table in tables %}
 {% if show_hidden or not table.hidden %}
 <div class="db-table">
-    <h2><a href="/{{ database }}-{{ database_hash }}/{{ table.name|quote_plus }}">{{ table.name }}</a>{% if table.hidden %}<em> (hidden)</em>{% endif %}</h2>
+    <h2><a href="/{{ database }}-{{ database_hash }}/{{ table.name|encode_table_name }}">{{ table.name }}</a>{% if table.hidden %}<em> (hidden)</em>{% endif %}</h2>
     <p><em>{% for column in table.columns[:9] %}{{ column }}{% if not loop.last %}, {% endif %}{% endfor %}{% if table.columns|length > 9 %}...{% endif %}</em></p>
     <p>{{ "{:,}".format(table.count) }} row{% if table.count == 1 %}{% else %}s{% endif %}</p>
 </div>
diff --git a/datasette/templates/index.html b/datasette/templates/index.html
index cb52740a..a32bf765 100644
--- a/datasette/templates/index.html
+++ b/datasette/templates/index.html
@@ -21,7 +21,7 @@
             {{ "{:,}".format(database.views_count) }} view{% if database.views_count != 1 %}s{% endif %}
         {% endif %}
     </p>
-    <p>{% for table in database.tables_truncated %}<a href="{{ database.path }}/{{ table.name|quote_plus }}" title="{{ table.count }} rows">{{ table.name }}</a>{% if not loop.last %}, {% endif %}{% endfor %}{% if database.tables_more %}, <a href="{{ database.path }}">...</a>{% endif %}</p>
+    <p>{% for table in database.tables_truncated %}<a href="{{ database.path }}/{{ table.name|encode_table_name }}" title="{{ table.count }} rows">{{ table.name }}</a>{% if not loop.last %}, {% endif %}{% endfor %}{% if database.tables_more %}, <a href="{{ database.path }}">...</a>{% endif %}</p>
 {% endfor %}
 
 {% endblock %}
diff --git a/datasette/templates/row.html b/datasette/templates/row.html
index 4ef0b758..df81899f 100644
--- a/datasette/templates/row.html
+++ b/datasette/templates/row.html
@@ -16,7 +16,7 @@
 {% block body_class %}row db-{{ database|to_css_class }} table-{{ table|to_css_class }}{% endblock %}
 
 {% block content %}
-<div class="hd"><a href="/">home</a> / <a href="/{{ database }}-{{ database_hash }}">{{ database }}</a> / <a href="/{{ database }}-{{ database_hash }}/{{ table|quote_plus }}">{{ table }}</a></div>
+<div class="hd"><a href="/">home</a> / <a href="/{{ database }}-{{ database_hash }}">{{ database }}</a> / <a href="/{{ database }}-{{ database_hash }}/{{ table|encode_table_name }}">{{ table }}</a></div>
 
 <h1 style="padding-left: 10px; border-left: 10px solid #{{ database_hash[:6] }}">{{ table }}: {{ ', '.join(primary_key_values) }}</a></h1>
 
@@ -31,7 +31,7 @@
     <ul>
         {% for other in foreign_key_tables %}
             <li>
-                <a href="/{{ database }}-{{ database_hash }}/{{ other.other_table|quote_plus }}?{{ other.other_column }}={{ ', '.join(primary_key_values) }}">
+                <a href="/{{ database }}-{{ database_hash }}/{{ other.other_table|encode_table_name }}?{{ other.other_column }}={{ ', '.join(primary_key_values) }}">
                     {{ "{:,}".format(other.count) }} row{% if other.count == 1 %}{% else %}s{% endif %}</a>
                 from {{ other.other_column }} in {{ other.other_table }}
             </li>
diff --git a/datasette/templates/table.html b/datasette/templates/table.html
index a768a9fc..978f62ab 100644
--- a/datasette/templates/table.html
+++ b/datasette/templates/table.html
@@ -29,7 +29,7 @@
     </h3>
 {% endif %}
 
-<form class="filters" action="/{{ database }}-{{ database_hash }}/{{ table|quote_plus }}" method="get">
+<form class="filters" action="/{{ database }}-{{ database_hash }}/{{ table|encode_table_name }}" method="get">
     {% if supports_search %}
         <div class="search-row"><label for="_search">Search:</label><input id="_search" type="search" name="_search" value="{{ search }}"></div>
     {% endif %}
diff --git a/datasette/utils.py b/datasette/utils.py
index b0f74f0d..6407857d 100644
--- a/datasette/utils.py
+++ b/datasette/utils.py
@@ -72,6 +72,23 @@ def urlsafe_components(token):
     ]
 
 
+_decode_table_name_re = re.compile(r"U\+([\da-h]{4})", re.IGNORECASE)
+_encode_table_name_re = re.compile("[{}]".format(''.join(re.escape(c) for c in (
+    ";", "/", "?", ":", "@", "&", "=", "+", "$", ",", "~"
+))))
+
+
+def decode_table_name(table_name):
+    return _decode_table_name_re.sub(lambda m: chr(int(m.group(1), 16)), table_name)
+
+
+def encode_table_name(table_name):
+    return _encode_table_name_re.sub(
+        lambda m: "U+{0:0{1}x}".format(ord(m.group(0)), 4).upper(),
+        table_name
+    )
+
+
 def path_from_row_pks(row, pks, use_rowid, quote=True):
     """ Generate an optionally URL-quoted unique identifier
         for a row from its primary keys."""
diff --git a/datasette/views/base.py b/datasette/views/base.py
index 119b376b..e9d8f14f 100644
--- a/datasette/views/base.py
+++ b/datasette/views/base.py
@@ -19,6 +19,8 @@ from datasette.utils import (
     InterruptedError,
     InvalidSql,
     LimitedWriter,
+    encode_table_name,
+    decode_table_name,
     is_url,
     path_from_row_pks,
     path_with_added_args,
@@ -161,7 +163,7 @@ class BaseView(RenderMixin):
         if expected != hash:
             if "table_and_format" in kwargs:
                 table, _format = resolve_table_and_format(
-                    table_and_format=urllib.parse.unquote_plus(
+                    table_and_format=decode_table_name(
                         kwargs["table_and_format"]
                     ),
                     table_exists=lambda t: self.ds.table_exists(name, t)
@@ -170,13 +172,13 @@ class BaseView(RenderMixin):
                 if _format:
                     kwargs["as_format"] = ".{}".format(_format)
             elif "table" in kwargs:
-                kwargs["table"] = urllib.parse.unquote_plus(
+                kwargs["table"] = decode_table_name(
                     kwargs["table"]
                 )
 
             should_redirect = "/{}-{}".format(name, expected)
             if "table" in kwargs:
-                should_redirect += "/" + urllib.parse.quote_plus(
+                should_redirect += "/" + encode_table_name(
                     kwargs["table"]
                 )
             if "pk_path" in kwargs:
@@ -305,7 +307,7 @@ class BaseView(RenderMixin):
             _format = (kwargs.pop("as_format", None) or "").lstrip(".")
         if "table_and_format" in kwargs:
             table, _ext_format = resolve_table_and_format(
-                table_and_format=urllib.parse.unquote_plus(
+                table_and_format=decode_table_name(
                     kwargs["table_and_format"]
                 ),
                 table_exists=lambda t: self.ds.table_exists(database, t)
@@ -314,7 +316,7 @@ class BaseView(RenderMixin):
             kwargs["table"] = table
             del kwargs["table_and_format"]
         elif "table" in kwargs:
-            kwargs["table"] = urllib.parse.unquote_plus(
+            kwargs["table"] = decode_table_name(
                 kwargs["table"]
             )
 
diff --git a/datasette/views/table.py b/datasette/views/table.py
index cb744708..35c877e4 100644
--- a/datasette/views/table.py
+++ b/datasette/views/table.py
@@ -11,6 +11,7 @@ from datasette.utils import (
     InterruptedError,
     append_querystring,
     compound_keys_after_sql,
+    encode_table_name,
     escape_sqlite,
     filters_should_redirect,
     is_url,
@@ -147,7 +148,7 @@ class RowTableShared(BaseView):
                         "value": jinja2.Markup(
                             '<a href="/{database}/{table}/{flat_pks_quoted}">{flat_pks}</a>'.format(
                                 database=database,
-                                table=urllib.parse.quote_plus(table),
+                                table=encode_table_name(table),
                                 flat_pks=str(
                                     jinja2.escape(
                                         path_from_row_pks(row, pks, not pks, False)
@@ -187,7 +188,7 @@ class RowTableShared(BaseView):
                     )
                     display_value = jinja2.Markup(link_template.format(
                         database=database,
-                        table=urllib.parse.quote_plus(other_table),
+                        table=encode_table_name(other_table),
                         link_id=urllib.parse.quote_plus(str(value)),
                         id=str(jinja2.escape(value)),
                         label=str(jinja2.escape(label)),
diff --git a/tests/test_api.py b/tests/test_api.py
index 1ab236cf..7317da53 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -597,7 +597,7 @@ def test_table_shape_object_compound_primary_Key(app_client):
 
 
 def test_table_with_slashes_in_name(app_client):
-    response = app_client.get('/fixtures/table%2Fwith%2Fslashes.csv?_shape=objects&_format=json')
+    response = app_client.get('/fixtures/tableU+002FwithU+002Fslashes.csv?_shape=objects&_format=json')
     assert response.status == 200
     data = response.json
     assert data['rows'] == [{
@@ -897,7 +897,7 @@ def test_row(app_client):
 
 
 def test_row_strange_table_name(app_client):
-    response = app_client.get('/fixtures/table%2Fwith%2Fslashes.csv/3.json?_shape=objects')
+    response = app_client.get('/fixtures/tableU+002FwithU+002Fslashes.csv/3.json?_shape=objects')
     assert response.status == 200
     assert [{'pk': '3', 'content': 'hey'}] == response.json['rows']
 
diff --git a/tests/test_html.py b/tests/test_html.py
index 913eac94..cee392e7 100644
--- a/tests/test_html.py
+++ b/tests/test_html.py
@@ -54,14 +54,14 @@ def test_row(app_client):
 
 def test_row_strange_table_name(app_client):
     response = app_client.get(
-        '/fixtures/table%2Fwith%2Fslashes.csv/3',
+        '/fixtures/tableU+002FwithU+002Fslashes.csv/3',
         allow_redirects=False
     )
     assert response.status == 302
     assert response.headers['Location'].endswith(
-        '/table%2Fwith%2Fslashes.csv/3'
+        '/tableU+002FwithU+002Fslashes.csv/3'
     )
-    response = app_client.get('/fixtures/table%2Fwith%2Fslashes.csv/3')
+    response = app_client.get('/fixtures/tableU+002FwithU+002Fslashes.csv/3')
     assert response.status == 200
 
 
@@ -358,7 +358,7 @@ def test_facets_persist_through_filter_form(app_client):
     ('/fixtures/simple_primary_key', [
         'table', 'db-fixtures', 'table-simple_primary_key'
     ]),
-    ('/fixtures/table%2Fwith%2Fslashes.csv', [
+    ('/fixtures/tableU+002FwithU+002Fslashes.csv', [
         'table', 'db-fixtures', 'table-tablewithslashescsv-fa7563'
     ]),
     ('/fixtures/simple_primary_key/1', [
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 43e2f44b..7002c1a5 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -374,3 +374,16 @@ def test_path_with_format(path, format, extra_qs, expected):
     )
     actual = utils.path_with_format(request, format, extra_qs)
     assert expected == actual
+
+
+@pytest.mark.parametrize("name,expected", [
+    ("table", "table"),
+    ("table/and/slashes", "tableU+002FandU+002Fslashes"),
+    ("~table", "U+007Etable"),
+    ("+bobcats!", "U+002Bbobcats!"),
+    ("U+007Etable", "UU+002B007Etable"),
+])
+def test_encode_decode_table_name(name, expected):
+    encoded = utils.encode_table_name(name)
+    assert encoded == expected
+    assert name == utils.decode_table_name(encoded)