get_container_info may lose all sysmeta

Bug #1765679 reported by Yuxin Wang
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Low
Unassigned
Swift3
In Progress
Undecided
Yuxin Wang

Bug Description

Per https://github.com/openstack/swift3/blob/1.12/swift3/request.py#L1263-L1266,
when the request had not authenticated yet, it would send a HEAD subrequest and then process by headers_to_container_info with resp.sw_headers. It will lose all sysmeta stored in resp.sysmeta_headers.

swift3 version is 1.12

Following are pdb outputs:

> /usr/lib/python2.7/site-packages/swift3/request.py(1256)get_container_info()
-> if is_success(info['status']):
(Pdb) l
1251 if self.is_authenticated:
1252 # if we have already authenticated, yes we can use the account
1253 # name like as AUTH_xxx for performance efficiency
1254 sw_req = self.to_swift_req(app, self.container_name, None)
1255 info = get_container_info(sw_req.environ, app)
1256 -> if is_success(info['status']):
1257 return info
1258 elif info['status'] == 404:
1259 raise NoSuchBucket(self.container_name)
1260 else:
1261 raise InternalError(
(Pdb) resp = self.get_response(app, 'HEAD', self.container_name, '')
(Pdb) info2 = headers_to_container_info(resp.sw_headers, resp.status_int)
(Pdb) info
{u'status': 204, u'sync_key': None, u'write_acl': None, u'versions': None, u'storage_policy': 0, u'bytes': 0, u'meta': {}, u'cors': {u'allow_origin': None, u'expose_headers': None, u'max_age': None}, u'sysmeta': {u'versions-mode': 'history', u'versions-location': 'test112+versioning', u'swift3-acl': '{"Owner":"s3_proj:s3test","Grant":[{"Grantee":"s3_proj:s3test","Permission":"FULL_CONTROL"}]}'}, u'object_count': 0, u'read_acl': None}
(Pdb) info2
{'status': 204, 'sync_key': None, 'storage_policy': '0', 'meta': {}, 'cors': {'allow_origin': None, 'expose_headers': None, 'max_age': None}, 'sysmeta': {'versions-mode': 'history', 'versions-location': 'test112+versioning'}, 'read_acl': None, 'object_count': '0', 'write_acl': None, 'versions': None, 'bytes': '0'}

swift3-acl is missing in the sysmeta of info2*.

*info2 is the return value of the else clause.

Yuxin Wang (chhyx2008)
Changed in swift3:
assignee: nobody → Yuxin Wang (chhyx2008)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift3 (master)

Fix proposed to branch: master
Review: https://review.openstack.org/563564

Changed in swift3:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on swift3 (master)

Change abandoned by Yuxin Wang (<email address hidden>) on branch: master
Review: https://review.openstack.org/563564
Reason: Need dependences.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift3 (master)

Fix proposed to branch: master
Review: https://review.openstack.org/564170

Revision history for this message
Tim Burke (1-tim-z) wrote :

Also affects s3api. More generally, I'm pretty sure get_container_info will give you (and presumably cache!!) radically different results depending upon where in the pipeline it was called...

Revision history for this message
Tim Burke (1-tim-z) wrote :

I finally "get" this bug! It took me until I tried to pick up https://review.openstack.org/#/c/436568/ and address your (very astute) review comment to understand...

Good news is that the impact should be small than I'd initially feared -- the damage should be limited to code that calls swift3/s3api's get_container_info, and it shouldn't lead to any cache poisoning. s3_acl = True configs may still be affected... though I'm struggling to get true *bad behavior* as a result. Looks like master currently mainly uses req.get_container_info() to check HTTP status, not so much headers. (Though there are probably a lot of calls, particularly around s3_acl, where get_container_info would be *preferable* to an actual HEAD...)

Should still get fixed, and the bug seems likely to block some other work we're hoping to get done.

Changed in swift:
status: New → Incomplete
status: Incomplete → Confirmed
importance: Undecided → Low
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on swift3 (master)

Change abandoned by Tim Burke (<email address hidden>) on branch: master
Review: https://review.openstack.org/564170
Reason: Thanks for this! I think I finally understand what's going on now -- took me long enough.

Targetted against s3api as https://review.openstack.org/#/c/629926/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/629926
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=a4298fb3ab071b1284fb7080f92c305c6b1cb445
Submitter: Zuul
Branch: master

commit a4298fb3ab071b1284fb7080f92c305c6b1cb445
Author: Wang Yuxin <email address hidden>
Date: Mon Apr 23 17:44:30 2018 +0800

    s3api: Fix get_container_info missing some sysmeta info.

    When get_container_info is called and not authenticated, it will
    make a HEAD subrequest and get info by passing resp.sw_headers to
    headers_to_container_info. This will lose all sysmeta stored in
    resp.sysmeta_headers.

    The patch fixes this by passing all sw_headers and
    sysmeta_headers to headers_to_container_info.

    Change-Id: I6e538ed7a748b60bdb9db7e894eaedc9d72559c1
    Closes-Bug: #1765679

Changed in swift:
status: Confirmed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/losf)

Fix proposed to branch: feature/losf
Review: https://review.opendev.org/655630

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/losf)
Download full text (11.3 KiB)

Reviewed: https://review.opendev.org/655630
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=d391957fd70a02866615c9c5edc070dbde2b23ea
Submitter: Zuul
Branch: feature/losf

commit 32bf43990c3c9ce771fca5968242f102155467c1
Author: ZhongShengping <chdzsp@163.com>
Date: Mon Apr 22 11:05:10 2019 +0800

    Replace git.openstack.org URLs with opendev.org URLs

    Thorough replacement of git.openstack.org URLs with their opendev.org
    counterparts.

    Change-Id: I5e77307de6a484fd69b2a5863423ceb357e8601f

commit 89eced960c5bf5c2e14b6245c70b615dc23d45a6
Author: OpenDev Sysadmins <email address hidden>
Date: Fri Apr 19 19:28:47 2019 +0000

    OpenDev Migration Patch

    This commit was bulk generated and pushed by the OpenDev sysadmins
    as a part of the Git hosting and code review systems migration
    detailed in these mailing list posts:

    http://lists.openstack.org/pipermail/openstack-discuss/2019-March/003603.html
    http://lists.openstack.org/pipermail/openstack-discuss/2019-April/004920.html

    Attempts have been made to correct repository namespaces and
    hostnames based on simple pattern matching, but it's possible some
    were updated incorrectly or missed entirely. Please reach out to us
    via the contact information listed at https://opendev.org/ with any
    questions you may have.

commit b6ebabee78b77108b2a3e5261873f038772d304d
Author: Tim Burke <email address hidden>
Date: Wed Apr 17 09:15:27 2019 -0700

    Clean up dlo unit tests

    We don't actually need so many py2/py3 branches, and once we
    clean those up, there's hardly any reason to import six.

    Change-Id: Ia3b4f02e7eb99ad1a76aa35c39dc198528fd39ad

commit 893acffbc01ba817361f8d8735513dc784fb92c0
Author: Pete Zaitcev <email address hidden>
Date: Wed Feb 20 17:09:08 2019 -0600

    py3: port dlo

    Change-Id: I7236ddea0acde93d0789ad8affa76df0097a86aa

commit 7cb276da425592280cd3f522c06cfb7b02311bde
Author: Tim Burke <email address hidden>
Date: Tue Apr 2 17:42:21 2019 -0700

    Move from py35 to py37 for gating py3 jobs

    Looking at https://governance.openstack.org/tc/reference/runtimes/train.html
    OpenStack is no longer targetting Python 3.5, so it doesn't really make sense
    to gate on it. Move to Python 3.7, just to make it so we can go longer before
    having to do this shuffle again.

    Presumably, once we actually declare support for Python 3.6, we should
    make that job voting, too? We've had enough gate flaky-ness lately that
    I'm not keen on it.

    Change-Id: I9c9e5df76a6592ab9c512b7c1ba807f9a0053059

commit 65d3cb75f817b3683aa85b73239d606e29c73d1c
Author: zhufl <email address hidden>
Date: Mon Apr 15 15:12:30 2019 +0800

    Use assertIn to check whether substring is in str

    This is to use assertIn to check whether substring is in str,
    go get clearer failure message.

    Change-Id: I40aff29454c423389755a5330751d2f69a227a05

commit e5ff405ec967bfe19b3eaeaac5b5a32c95218f9d
Author: Tim Burke <email address hidden>
Date: Fri Jun 1 15:59:25 2018 -0700

    Make staticweb return URL-encoded Location headers

 ...

tags: added: in-feature-losf
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.22.0

This issue was fixed in the openstack/swift 2.22.0 release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.