From 133467a6c08f925b8058a52effad9d19a09033c9 Mon Sep 17 00:00:00 2001 From: Tyler Rockwood Date: Thu, 25 Oct 2018 05:12:57 -0700 Subject: [PATCH 1/2] 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. --- firebase_admin/db.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/firebase_admin/db.py b/firebase_admin/db.py index 2fcbbc809..d37a7fe88 100644 --- a/firebase_admin/db.py +++ b/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 From bf96c73efdd561e003cbe4cb9b00c3835be2b837 Mon Sep 17 00:00:00 2001 From: Tyler Rockwood Date: Tue, 20 Nov 2018 09:08:03 -0800 Subject: [PATCH 2/2] Add tests for X-Firebase-Decoding --- tests/test_db.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_db.py b/tests/test_db.py index 6168b72d4..b42123d34 100644 --- a/tests/test_db.py +++ b/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):