Middleware that allows a user to have quoted Etags

Users have complained for a while that Swift's ETags don't match the
expected RFC formats. We've resisted fixing this for just as long,
worrying that the fix would break innumerable clients that expect the
value to be a hex-encoded MD5 digest and *nothing else*.

But, users keep asking for it, and some consumers (including some CDNs)
break if we *don't* have quoted etags -- so, let's make it an option.

With this middleware, Swift users can set metadata per-account or even
per-container to explicitly request RFC compliant etags or not. Swift
operators also get an option to change the default behavior
cluster-wide; it defaults to the old, non-compliant format.

See also:
  - https://tools.ietf.org/html/rfc2616#section-3.11
  - https://tools.ietf.org/html/rfc7232#section-2.3

Closes-Bug: 1099087
Closes-Bug: 1424614
Co-Authored-By: Tim Burke <tim.burke@gmail.com>
Change-Id: I380c6e34949d857158e11eb428b3eda9975d855d

doc/source/logs.rst

@@ -141,6 +141,7 @@ SYM                     :ref:`symlink`
 SH                      :ref:`sharding_doc`
 S3                      :ref:`s3api`
 OV                      :ref:`object_versioning`
+EQ                      :ref:`etag_quoter`
 ======================= =============================
 
 

doc/source/middleware.rst

@@ -207,6 +207,15 @@ Encryption middleware should be deployed in conjunction with the
     :members:
     :show-inheritance:
 
+.. _etag_quoter:
+
+Etag Quoter
+===========
+
+.. automodule:: swift.common.middleware.etag_quoter
+    :members:
+    :show-inheritance:
+
 .. _formpost:
 
 FormPost

etc/proxy-server.conf-sample

@@ -850,6 +850,17 @@ use = egg:swift#name_check
 # maximum_length = 255
 # forbidden_regexp = /\./|/\.\./|/\.$|/\.\.$
 
+# Note: Etag quoter should be placed just after cache in the pipeline.
+[filter:etag-quoter]
+use = egg:swift#etag_quoter
+# Historically, Swift has emitted bare MD5 hex digests as ETags, which is not
+# RFC compliant. With this middleware in the pipeline, users can opt-in to
+# RFC-compliant ETags on a per-account or per-container basis.
+#
+# Set to true to enable RFC-compliant ETags cluster-wide by default. Users
+# can still opt-out by setting appropriate account or container metadata.
+# enable_by_default = false
+
 [filter:list-endpoints]
 use = egg:swift#list_endpoints
 # list_endpoints_path = /endpoints/

setup.cfg

@@ -126,6 +126,7 @@ paste.filter_factory =
     symlink = swift.common.middleware.symlink:filter_factory
     s3api = swift.common.middleware.s3api.s3api:filter_factory
     s3token = swift.common.middleware.s3api.s3token:filter_factory
+    etag_quoter = swift.common.middleware.etag_quoter:filter_factory
 
 swift.diskfile =
     replication.fs = swift.obj.diskfile:DiskFileManager

swift/common/middleware/etag_quoter.py

@@ -0,0 +1,127 @@
+# Copyright (c) 2010-2020 OpenStack Foundation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This middleware fix the Etag header of responses so that it is RFC compliant.
+`RFC 7232 <https://tools.ietf.org/html/rfc7232#section-2.3>`__ specifies that
+the value of the Etag header must be double quoted.
+
+It must be placed at the beggining of the pipeline, right after cache::
+
+   [pipeline:main]
+   pipeline = ...  cache etag-quoter ...
+
+   [filter:etag-quoter]
+   use = egg:swift#etag_quoter
+
+Set ``X-Account-Rfc-Compliant-Etags: true`` at the account
+level to have any Etags in object responses be double quoted, as in
+``"d41d8cd98f00b204e9800998ecf8427e"``. Alternatively, you may
+only fix Etags in a single container by setting
+``X-Container-Rfc-Compliant-Etags: true`` on the container.
+This may be necessary for Swift to work properly with some CDNs.
+
+Either option may also be explicitly *disabled*, so you may enable quoted
+Etags account-wide as above but turn them off for individual containers
+with ``X-Container-Rfc-Compliant-Etags: false``. This may be
+useful if some subset of applications expect Etags to be bare MD5s.
+"""
+
+from swift.common.constraints import valid_api_version
+from swift.common.http import is_success
+from swift.common.swob import Request
+from swift.common.utils import config_true_value, register_swift_info
+from swift.proxy.controllers.base import get_account_info, get_container_info
+
+
+class EtagQuoterMiddleware(object):
+    def __init__(self, app, conf):
+        self.app = app
+        self.conf = conf
+
+    def __call__(self, env, start_response):
+        req = Request(env)
+        try:
+            version, account, container, obj = req.split_path(
+                2, 4, rest_with_last=True)
+            is_swifty_request = valid_api_version(version)
+        except ValueError:
+            is_swifty_request = False
+
+        if not is_swifty_request:
+            return self.app(env, start_response)
+
+        if not obj:
+            typ = 'Container' if container else 'Account'
+            client_header = 'X-%s-Rfc-Compliant-Etags' % typ
+            sysmeta_header = 'X-%s-Sysmeta-Rfc-Compliant-Etags' % typ
+            if client_header in req.headers:
+                if req.headers[client_header]:
+                    req.headers[sysmeta_header] = config_true_value(
+                        req.headers[client_header])
+                else:
+                    req.headers[sysmeta_header] = ''
+            if req.headers.get(client_header.replace('X-', 'X-Remove-', 1)):
+                req.headers[sysmeta_header] = ''
+
+            def translating_start_response(status, headers, exc_info=None):
+                return start_response(status, [
+                    (client_header if h.title() == sysmeta_header else h,
+                     v) for h, v in headers
+                ], exc_info)
+
+            return self.app(env, translating_start_response)
+
+        container_info = get_container_info(env, self.app, 'EQ')
+        if not container_info or not is_success(container_info['status']):
+            return self.app(env, start_response)
+
+        flag = container_info.get('sysmeta', {}).get('rfc-compliant-etags')
+        if flag is None:
+            account_info = get_account_info(env, self.app, 'EQ')
+            if not account_info or not is_success(account_info['status']):
+                return self.app(env, start_response)
+
+            flag = account_info.get('sysmeta', {}).get(
+                'rfc-compliant-etags')
+
+        if flag is None:
+            flag = self.conf.get('enable_by_default', 'false')
+
+        if not config_true_value(flag):
+            return self.app(env, start_response)
+
+        status, headers, resp_iter = req.call_application(self.app)
+
+        for i, (header, value) in enumerate(headers):
+            if header.lower() == 'etag':
+                if not value.startswith(('"', 'W/"')) or \
+                        not value.endswith('"'):
+                    headers[i] = (header, '"%s"' % value)
+
+        start_response(status, headers)
+        return resp_iter
+
+
+def filter_factory(global_conf, **local_conf):
+    conf = global_conf.copy()
+    conf.update(local_conf)
+    register_swift_info(
+        'etag_quoter', enable_by_default=config_true_value(
+            conf.get('enable_by_default', 'false')))
+
+    def etag_quoter_filter(app):
+        return EtagQuoterMiddleware(app, conf)
+    return etag_quoter_filter

test/functional/test_object.py

@@ -16,6 +16,7 @@
 # limitations under the License.
 
 import datetime
+import hashlib
 import json
 import unittest
 from uuid import uuid4
@@ -1719,6 +1720,58 @@ class TestObject(unittest.TestCase):
         self.assertEqual(len(final_status[0].childNodes), 1)
         self.assertEqual(final_status[0].childNodes[0].data, '200 OK')
 
+    def test_etag_quoter(self):
+        if tf.skip:
+            raise SkipTest
+        if 'etag_quoter' not in tf.cluster_info:
+            raise SkipTest("etag-quoter middleware is not enabled")
+
+        def do_head(expect_quoted=False):
+            def head(url, token, parsed, conn):
+                conn.request('HEAD', '%s/%s/%s' % (
+                    parsed.path, self.container, self.obj), '',
+                    {'X-Auth-Token': token})
+                return check_response(conn)
+
+            resp = retry(head)
+            resp.read()
+            self.assertEqual(resp.status, 200)
+            expected_etag = hashlib.md5(b'test').hexdigest()
+            if expect_quoted:
+                expected_etag = '"%s"' % expected_etag
+            self.assertEqual(resp.headers['etag'], expected_etag)
+
+        def _post(enable_flag, container_path):
+            def post(url, token, parsed, conn):
+                if container_path:
+                    path = '%s/%s' % (parsed.path, self.container)
+                    hdr = 'X-Container-Rfc-Compliant-Etags'
+                else:
+                    path = parsed.path
+                    hdr = 'X-Account-Rfc-Compliant-Etags'
+                headers = {hdr: enable_flag, 'X-Auth-Token': token}
+                conn.request('POST', path, '', headers)
+                return check_response(conn)
+
+            resp = retry(post)
+            resp.read()
+            self.assertEqual(resp.status, 204)
+
+        def post_account(enable_flag):
+            return _post(enable_flag, False)
+
+        def post_container(enable_flag):
+            return _post(enable_flag, True)
+
+        do_head()
+        post_container('t')
+        do_head(expect_quoted=True)
+        post_account('t')
+        post_container('')
+        do_head(expect_quoted=True)
+        post_container('f')
+        do_head()
+
 
 if __name__ == '__main__':
     unittest.main()

test/unit/common/middleware/test_etag_quoter.py

@@ -0,0 +1,215 @@
+# Copyright (c) 2010-2020 OpenStack Foundation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import mock
+import unittest
+
+from swift.common import swob
+from swift.common.middleware import etag_quoter
+from swift.proxy.controllers.base import get_cache_key
+
+from test.unit.common.middleware.helpers import FakeSwift
+
+
+def set_info_cache(req, cache_data, account, container=None):
+    req.environ.setdefault('swift.infocache', {})[
+        get_cache_key(account, container)] = cache_data
+
+
+class TestEtagQuoter(unittest.TestCase):
+    def get_mw(self, conf, etag='unquoted-etag', path=None):
+        if path is None:
+            path = '/v1/AUTH_acc/con/some/path/to/obj'
+        app = FakeSwift()
+        hdrs = {} if etag is None else {'ETag': etag}
+        app.register('GET', path, swob.HTTPOk, hdrs)
+        return etag_quoter.filter_factory({}, **conf)(app)
+
+    @mock.patch('swift.common.middleware.etag_quoter.register_swift_info')
+    def test_swift_info(self, mock_register):
+        self.get_mw({})
+        self.assertEqual(mock_register.mock_calls, [
+            mock.call('etag_quoter', enable_by_default=False)])
+        mock_register.reset_mock()
+
+        self.get_mw({'enable_by_default': '1'})
+        self.assertEqual(mock_register.mock_calls, [
+            mock.call('etag_quoter', enable_by_default=True)])
+        mock_register.reset_mock()
+
+        self.get_mw({'enable_by_default': 'no'})
+        self.assertEqual(mock_register.mock_calls, [
+            mock.call('etag_quoter', enable_by_default=False)])
+
+    def test_account_on_overrides_cluster_off(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {'rfc-compliant-etags': '1'},
+        }, 'AUTH_acc')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {},
+        }, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 'false'}))
+        self.assertEqual(resp.headers['ETag'], '"unquoted-etag"')
+
+    def test_account_off_overrides_cluster_on(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {'rfc-compliant-etags': 'no'},
+        }, 'AUTH_acc')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {},
+        }, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 'yes'}))
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+
+    def test_container_on_overrides_cluster_off(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {},
+        }, 'AUTH_acc')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {'rfc-compliant-etags': 't'},
+        }, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 'false'}))
+        self.assertEqual(resp.headers['ETag'], '"unquoted-etag"')
+
+    def test_container_off_overrides_cluster_on(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {},
+        }, 'AUTH_acc')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {'rfc-compliant-etags': '0'},
+        }, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 'yes'}))
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+
+    def test_container_on_overrides_account_off(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {'rfc-compliant-etags': 'no'},
+        }, 'AUTH_acc')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {'rfc-compliant-etags': 't'},
+        }, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({}))
+        self.assertEqual(resp.headers['ETag'], '"unquoted-etag"')
+
+    def test_container_off_overrides_account_on(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {'rfc-compliant-etags': 'yes'},
+        }, 'AUTH_acc')
+        set_info_cache(req, {
+            'status': 200,
+            'sysmeta': {'rfc-compliant-etags': 'false'},
+        }, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({}))
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+
+    def test_cluster_wide(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 't'}))
+        self.assertEqual(resp.headers['ETag'], '"unquoted-etag"')
+
+    def test_already_valid(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 't'},
+                                            '"quoted-etag"'))
+        self.assertEqual(resp.headers['ETag'], '"quoted-etag"')
+
+    def test_already_weak_but_valid(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 't'},
+                                            'W/"weak-etag"'))
+        self.assertEqual(resp.headers['ETag'], 'W/"weak-etag"')
+
+    def test_only_half_valid(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 't'},
+                                            '"weird-etag'))
+        self.assertEqual(resp.headers['ETag'], '""weird-etag"')
+
+    def test_no_etag(self):
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc', 'con')
+        resp = req.get_response(self.get_mw({'enable_by_default': 't'},
+                                            etag=None))
+        self.assertNotIn('ETag', resp.headers)
+
+    def test_non_swift_path(self):
+        path = '/some/other/location/entirely'
+        req = swob.Request.blank(path)
+        resp = req.get_response(self.get_mw({'enable_by_default': 't'},
+                                            path=path))
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+
+    def test_non_object_request(self):
+        path = '/v1/AUTH_acc/con'
+        req = swob.Request.blank(path)
+        resp = req.get_response(self.get_mw({'enable_by_default': 't'},
+                                            path=path))
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+
+    def test_no_container_info(self):
+        mw = self.get_mw({'enable_by_default': 't'})
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc')
+        mw.app.register('HEAD', '/v1/AUTH_acc/con',
+                        swob.HTTPServiceUnavailable, {})
+        resp = req.get_response(mw)
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+        set_info_cache(req, {'status': 404, 'sysmeta': {}}, 'AUTH_acc', 'con')
+        resp = req.get_response(mw)
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc', 'con')
+        resp = req.get_response(mw)
+        self.assertEqual(resp.headers['ETag'], '"unquoted-etag"')
+
+    def test_no_account_info(self):
+        mw = self.get_mw({'enable_by_default': 't'})
+        req = swob.Request.blank('/v1/AUTH_acc/con/some/path/to/obj')
+        mw.app.register('HEAD', '/v1/AUTH_acc',
+                        swob.HTTPServiceUnavailable, {})
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc', 'con')
+        resp = req.get_response(mw)
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+        set_info_cache(req, {'status': 404, 'sysmeta': {}}, 'AUTH_acc')
+        resp = req.get_response(mw)
+        self.assertEqual(resp.headers['ETag'], 'unquoted-etag')
+        set_info_cache(req, {'status': 200, 'sysmeta': {}}, 'AUTH_acc')
+        resp = req.get_response(mw)
+        self.assertEqual(resp.headers['ETag'], '"unquoted-etag"')