Add header for opting into correct URL decoding (#206)

* Add header for opting into correct URL decoding

There is a long standing bug (~years) in the RTDB url decoding where we URL decode query parameters twice. Since we can't simply fix the issue without breaking users, we allow an opt-in upgrade/fix of the correct behavior via a header `X-Firebase-Decoding: 1`. We hope to make this the default (correct) behavior at some point in the near future. Which then this header will not be needed anymore.

* Add tests for X-Firebase-Decoding

Author: rockwotj

firebase_admin/db.py

@@ -36,7 +36,7 @@
 
 
 _DB_ATTRIBUTE = '_database'
-_INVALID_PATH_CHARACTERS = '[].?#$'
+_INVALID_PATH_CHARACTERS = '[].#$'
 _RESERVED_FILTERS = ('$key', '$value', '$priority')
 _USER_AGENT = 'Firebase/HTTP/{0}/{1}.{2}/AdminPython'.format(
     firebase_admin.__version__, sys.version_info.major, sys.version_info.minor)
@@ -845,7 +845,8 @@ def __init__(self, credential, base_url, auth_override, timeout):
               timeout, which is the default behavior of the underlying requests library.
         """
         _http_client.JsonHttpClient.__init__(
-            self, credential=credential, base_url=base_url, headers={'User-Agent': _USER_AGENT})
+            self, credential=credential, base_url=base_url,
+            headers={'User-Agent': _USER_AGENT, 'X-Firebase-Decoding': '1'})
         self.credential = credential
         self.auth_override = auth_override
         self.timeout = timeout

tests/test_db.py

@@ -157,6 +157,7 @@ def test_get_value(self, data):
         assert recorder[0].url == 'https://test.firebaseio.com/test.json'
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
         assert 'X-Firebase-ETag' not in recorder[0].headers
 
     @pytest.mark.parametrize('data', valid_values)
@@ -169,6 +170,7 @@ def test_get_with_etag(self, data):
         assert recorder[0].url == 'https://test.firebaseio.com/test.json'
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
         assert recorder[0].headers['X-Firebase-ETag'] == 'true'
 
     @pytest.mark.parametrize('data', valid_values)
@@ -180,6 +182,7 @@ def test_get_shallow(self, data):
         assert recorder[0].method == 'GET'
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?shallow=true'
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
 
     def test_get_with_etag_and_shallow(self):
@@ -197,12 +200,14 @@ def test_get_if_changed(self, data):
         assert recorder[0].method == 'GET'
         assert recorder[0].url == 'https://test.firebaseio.com/test.json'
         assert recorder[0].headers['if-none-match'] == 'invalid-etag'
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
         assert ref.get_if_changed(MockAdapter.ETAG) == (False, None, None)
         assert len(recorder) == 2
         assert recorder[1].method == 'GET'
         assert recorder[1].url == 'https://test.firebaseio.com/test.json'
         assert recorder[1].headers['if-none-match'] == MockAdapter.ETAG
+        assert recorder[1].headers['X-Firebase-Decoding'] == '1'
 
     @pytest.mark.parametrize('etag', [0, 1, True, False, dict(), list(), tuple()])
     def test_get_if_changed_invalid_etag(self, etag):
@@ -221,6 +226,7 @@ def test_order_by_query(self, data):
         assert recorder[0].method == 'GET'
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?' + query_str
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     @pytest.mark.parametrize('data', valid_values)
     def test_limit_query(self, data):
@@ -234,6 +240,7 @@ def test_limit_query(self, data):
         assert recorder[0].method == 'GET'
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?' + query_str
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     @pytest.mark.parametrize('data', valid_values)
     def test_range_query(self, data):
@@ -248,6 +255,7 @@ def test_range_query(self, data):
         assert recorder[0].method == 'GET'
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?' + query_str
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     @pytest.mark.parametrize('data', valid_values)
     def test_set_value(self, data):
@@ -259,6 +267,7 @@ def test_set_value(self, data):
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?print=silent'
         assert json.loads(recorder[0].body.decode()) == data
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     def test_set_none_value(self):
         ref = db.reference('/test')
@@ -285,6 +294,7 @@ def test_update_children(self, data):
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?print=silent'
         assert json.loads(recorder[0].body.decode()) == data
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     @pytest.mark.parametrize('data', valid_values)
     def test_set_if_unchanged_success(self, data):
@@ -298,6 +308,7 @@ def test_set_if_unchanged_success(self, data):
         assert json.loads(recorder[0].body.decode()) == data
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['if-match'] == MockAdapter.ETAG
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     @pytest.mark.parametrize('data', valid_values)
     def test_set_if_unchanged_failure(self, data):
@@ -311,6 +322,7 @@ def test_set_if_unchanged_failure(self, data):
         assert json.loads(recorder[0].body.decode()) == data
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['if-match'] == 'invalid-etag'
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     @pytest.mark.parametrize('etag', [0, 1, True, False, dict(), list(), tuple()])
     def test_set_if_unchanged_invalid_etag(self, etag):
@@ -356,6 +368,7 @@ def test_push(self, data):
         assert json.loads(recorder[0].body.decode()) == data
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     def test_push_default(self):
         ref = db.reference('/test')
@@ -367,6 +380,7 @@ def test_push_default(self):
         assert json.loads(recorder[0].body.decode()) == ''
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     def test_push_none_value(self):
         ref = db.reference('/test')
@@ -383,6 +397,7 @@ def test_delete(self):
         assert recorder[0].url == 'https://test.firebaseio.com/test.json'
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     def test_transaction(self):
         ref = db.reference('/test')
@@ -568,6 +583,7 @@ def test_get_value(self):
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?' + query_str
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     def test_set_value(self):
         ref = db.reference('/test')
@@ -581,6 +597,7 @@ def test_set_value(self):
         assert json.loads(recorder[0].body.decode()) == data
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     def test_order_by_query(self):
         ref = db.reference('/test')
@@ -593,6 +610,7 @@ def test_order_by_query(self):
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?' + query_str
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
     def test_range_query(self):
         ref = db.reference('/test')
@@ -606,6 +624,7 @@ def test_range_query(self):
         assert recorder[0].url == 'https://test.firebaseio.com/test.json?' + query_str
         assert recorder[0].headers['Authorization'] == 'Bearer mock-token'
         assert recorder[0].headers['User-Agent'] == db._USER_AGENT
+        assert recorder[0].headers['X-Firebase-Decoding'] == '1'
 
 
 class TestDatabaseInitialization(object):