OpenStack Object Storage (swift)

Invalid account acl header value generates 500 response

Bug #1281626 reported by Alistair Coles on 2014-02-18

This bug affects 2 people

Affects		Status	Importance	Assigned to	Milestone
	OpenStack Object Storage (swift)	Fix Released	Undecided	Alistair Coles	OpenStack Object Storage (swift) 1.13.0

Bug Description

Sending an X-Account-Access-Control header with value that is valid json but not a valid account ACL sometimes results in a 500 Internal Error being returned, other times a 400 Bad Request is returned. Should always be a 400.

For example:
$ curl http://myhost:8080/v1/AUTH_test -H "X-Auth-Token:mytoken" -X POST -i -H 'X-Account-Access-Control:["admin"]'
HTTP/1.1 500 Internal Error
Content-Length: 125
Content-Type: text/html; charset=UTF-8
X-Trans-Id: tx8e9582c85d5f4ff98e494-0053037731
Date: Tue, 18 Feb 2014 15:07:29 GMT

<html><h1>Internal Error</h1><p>The server has either erred or is incapable of performing the requested operation.</p></html>

$ curl http://myhost:8080/v1/AUTH_test -H "X-Auth-Token:mytoken" -X POST -i -H 'X-Account-Access-Control:2'
HTTP/1.1 500 Internal Error
Content-Length: 125
Content-Type: text/html; charset=UTF-8
X-Trans-Id: tx9015d27661e94e0487132-005303776f
Date: Tue, 18 Feb 2014 15:08:31 GMT

<html><h1>Internal Error</h1><p>The server has either erred or is incapable of performing the requested operation.</p></html>

$ curl http://myhost:8080/v1/AUTH_test -H "X-Auth-Token:mytoken" -X POST -i -H 'X-Account-Access-Control:{"admin":2}'
HTTP/1.1 400 Bad Request
Content-Length: 91
Content-Type: text/plain; charset=UTF-8
X-Trans-Id: tx911dc715f7f3410f8f03c-0053037c90
Date: Tue, 18 Feb 2014 15:30:24 GMT

X-Account-Access-Control invalid: Value for key 'admin' must be a list

Input: {"admin":2}

Alistair Coles (alistair-coles) on 2014-02-18

Changed in swift:
assignee:	nobody → Alistair Coles (alistair-coles)

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2014-02-18: Fix proposed to swift (master)

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

Changed in swift:
status:	New → In Progress

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2014-02-23: Fix merged to swift (master)

Reviewed: https://review.openstack.org/74417
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=36adcb6c66bf0c0093ead1080078ca8154b0d158
Submitter: Jenkins
Branch: master

commit 36adcb6c66bf0c0093ead1080078ca8154b0d158
Author: anc <email address hidden>
Date: Tue Feb 18 14:07:37 2014 +0000

Fix invalid account acl generating 500 response.

    Sending an account POST with an X-Account-Access-Control
    header value that is valid json but not a valid ACL was
    causing a 500 Internal Error if the value did not parse
    to a dict due to an exception being raised in tempauth.py.

    This patch modifies acl.py to check that the header value
    is both json and parses to a dict. The existing
    tests are extended to cover these invalid header values.

    This patch also enables json encoded dicts with whitespace
    (e.g. '{ }') to be accepted as a value for
    X-Account-Access-Control in the same way that '{}' is.
    These previously resulted in a 400 response.

Closes-bug: 1281626

Change-Id: Ia06ba9c9d16f749f801a8158e73d3898c4a42888

Changed in swift:
status:	In Progress → Fix Committed

Thierry Carrez (ttx) on 2014-02-25

Changed in swift:
milestone:	none → 1.13.0
status:	Fix Committed → Fix Released

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2014-02-28: Fix proposed to swift (feature/ec)

Fix proposed to branch: feature/ec
Review: https://review.openstack.org/77244

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2014-03-01: Fix merged to swift (feature/ec)

Download full text (12.9 KiB)

Reviewed: https://review.openstack.org/77244
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=2c22fb0a17e1192b95d0b823dfce144779579944
Submitter: Jenkins
Branch: feature/ec

commit dc5c5fb4011cc8dee9b149e668dc8534ac7f51e0
Author: Hou Ming Wang <email address hidden>
Date: Wed Feb 26 20:53:10 2014 +0800

Remove blank space after print

Check Swift code, remove blank space after print.

Change-Id: Ia7838ac5f33f2b335d1cd664b017e87118262d29

commit b6eda4a40bd55771771ca8039e9c3ad39ff7de11
Author: John Dickinson <email address hidden>
Date: Sun Feb 23 15:24:47 2014 -0800

AUTHORS and CHANGELOG update for 1.13 release

Change-Id: I6195c70cbbd7c94682539962dd5410e3665f01ac

commit 3357a58ea2a8fc35e6672c52028e59fbafe7fcb7
Author: Matt Kassawara <email address hidden>
Date: Thu Feb 20 20:27:20 2014 -0700

Alphabetized items under "Middleware" in source documentation

I alphabetized the items under "Middleware" in the source documentation
to make them easier to locate.

Change-Id: I3a0108c89d16ef07b7623dda518b3096c2686002

commit beaa2b8744841acac5af1a2171a358f58d7288be
Author: John Dickinson <email address hidden>
Date: Sun Feb 23 10:22:34 2014 -0800

tabs to spaces

Change-Id: Ie3d3af87df8f72792abf345c0d4dcfe2be1f4141

commit f42cb54002eaaa1f82a7a489802afc85412e2ce1
Author: Samuel Merritt <email address hidden>
Date: Fri Feb 21 16:49:22 2014 -0800

Refactor a quorum-size calculation

No behavior change, just use the utility function instead of rolling
our own.

Change-Id: I152ccd8f22f424cd7547fa4d67df020f263096a9

commit 5dfa0bd3fba6fe414f7a6ed9d7d19f9dea25445e
Author: Matt Kassawara <email address hidden>
Date: Fri Feb 21 10:59:29 2014 -0700

Alphabetized items under other sections in source documentation

    I alphabetized the items under "Proxy", "Account", "Container",
    and "Object" in the source documentation to make them easier to
    locate.

Change-Id: Ia9cca0ee558cb1e0361c1a88103352bd006da1e3

commit 38e015e871add82889d1e7999b35b795f4a7d1f5
Author: Samuel Merritt <email address hidden>
Date: Thu Feb 20 23:01:00 2014 -0800

Functional tests for DLO If-Match/If-None-Match

Change-Id: I4a731a3836eb0f58b897ee43477b3b76d2344a81

commit 46250171ef285b0d297b05f413f1a27ff1dbdb65
Author: Samuel Merritt <email address hidden>
Date: Thu Feb 20 22:01:39 2014 -0800

Functional tests for SLO If-Match/If-None-Match

Change-Id: Ib70f579f5edebc7b1586e5f51fee5769bcaa9931

commit 25ebf3aa9e6e98974565ae1b70fc05b49423e17c
Author: Samuel Merritt <email address hidden>
Date: Thu Feb 20 20:33:00 2014 -0800

Raise error on long or short DLO

    The GET response for a DLO has a Content-Length that's computed from
    the container listing, but the response body's length is determined by
    the segments. If a segment grows or shrinks between when the headers
    are sent and when the segment is requested, this confuses clients.

    For example, if the DLO is longer than its Content-Length and the
    client sends another request on the same TCP connection, then it can
    get leftover object bytes instea...

Reviewed:  https://review.openstack.org/77244
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=2c22fb0a17e1192b95d0b823dfce144779579944
Submitter: Jenkins
Branch:    feature/ec

commit dc5c5fb4011cc8dee9b149e668dc8534ac7f51e0
Author: Hou Ming Wang <houming.wang@easystack.cn>
Date:   Wed Feb 26 20:53:10 2014 +0800

Remove blank space after print
    
    Check Swift code, remove blank space after print.
    
    Change-Id: Ia7838ac5f33f2b335d1cd664b017e87118262d29

commit b6eda4a40bd55771771ca8039e9c3ad39ff7de11
Author: John Dickinson <me@not.mn>
Date:   Sun Feb 23 15:24:47 2014 -0800

AUTHORS and CHANGELOG update for 1.13 release
    
    Change-Id: I6195c70cbbd7c94682539962dd5410e3665f01ac

commit 3357a58ea2a8fc35e6672c52028e59fbafe7fcb7
Author: Matt Kassawara <mkassawara@gmail.com>
Date:   Thu Feb 20 20:27:20 2014 -0700

Alphabetized items under "Middleware" in source documentation
    
    I alphabetized the items under "Middleware" in the source documentation
    to make them easier to locate.
    
    Change-Id: I3a0108c89d16ef07b7623dda518b3096c2686002

commit beaa2b8744841acac5af1a2171a358f58d7288be
Author: John Dickinson <me@not.mn>
Date:   Sun Feb 23 10:22:34 2014 -0800

tabs to spaces
    
    Change-Id: Ie3d3af87df8f72792abf345c0d4dcfe2be1f4141

commit f42cb54002eaaa1f82a7a489802afc85412e2ce1
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Fri Feb 21 16:49:22 2014 -0800

Refactor a quorum-size calculation
    
    No behavior change, just use the utility function instead of rolling
    our own.
    
    Change-Id: I152ccd8f22f424cd7547fa4d67df020f263096a9

commit 5dfa0bd3fba6fe414f7a6ed9d7d19f9dea25445e
Author: Matt Kassawara <mkassawara@gmail.com>
Date:   Fri Feb 21 10:59:29 2014 -0700

Alphabetized items under other sections in source documentation
    
    I alphabetized the items under "Proxy", "Account", "Container",
    and "Object" in the source documentation to make them easier to
    locate.
    
    Change-Id: Ia9cca0ee558cb1e0361c1a88103352bd006da1e3

commit 38e015e871add82889d1e7999b35b795f4a7d1f5
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Thu Feb 20 23:01:00 2014 -0800

Functional tests for DLO If-Match/If-None-Match
    
    Change-Id: I4a731a3836eb0f58b897ee43477b3b76d2344a81

commit 46250171ef285b0d297b05f413f1a27ff1dbdb65
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Thu Feb 20 22:01:39 2014 -0800

Functional tests for SLO If-Match/If-None-Match
    
    Change-Id: Ib70f579f5edebc7b1586e5f51fee5769bcaa9931

commit 25ebf3aa9e6e98974565ae1b70fc05b49423e17c
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Thu Feb 20 20:33:00 2014 -0800

Raise error on long or short DLO
    
    The GET response for a DLO has a Content-Length that's computed from
    the container listing, but the response body's length is determined by
    the segments. If a segment grows or shrinks between when the headers
    are sent and when the segment is requested, this confuses clients.
    
    For example, if the DLO is longer than its Content-Length and the
    client sends another request on the same TCP connection, then it can
    get leftover object bytes instead of an HTTP status line.
    Alternately, if the headers it sends fill up the TCP buffers since
    Swift won't read them until the first response is done, then deadlock
    hilarity ensues.
    
    If the DLO is shorter than its Content-Length, you're pretty much
    guaranteed a deadlock: the client is waiting for the rest of the
    response, and the server is waiting for a new request.
    
    Now SegmentedIterable detects both these conditions and raises an
    exception so that the TCP connection gets torn down. It can't save
    this request, but it can stop the next one from getting hosed too.
    
    Change-Id: Icf79ba046ef7aaaab49ce6d0b33147332c967afc

commit 05854c51340f80fe56fec64ca54586c33717f1f4
Author: Matt Kassawara <mkassawara@gmail.com>
Date:   Thu Feb 20 19:19:09 2014 -0700

Alphabetized items under "Misc" in source documentation
    
    I alphabetized the items under "Misc" in the source documentation to
    make them easier to locate.
    
    Change-Id: I1badca7a97e7d887fefac0be2e2208eec52cec60

commit 1f67eb7403e6000840c44ec702c406f177ea1942
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Wed Feb 12 18:29:12 2014 -0800

Support If-[None-]Match for object HEAD, SLO, and DLO
    
    I moved the checking of If-Match and If-None-Match out of the object
    server's GET method and into swob so that everyone can use it. The
    interface is similar to the Range handling; make a response with
    conditional_response=True, and you get handing of If-Match and
    If-None-Match.
    
    Since the only users of conditional_response are object GET, object
    HEAD, SLO, and DLO, this has the effect of adding support for If-Match
    and If-None-Match to just the latter three places and nowhere
    else. This makes object GET and HEAD consistent for any kind of
    object, large or small.
    
    This also fixes a bug where various conditional headers (If-*) were
    passed through to the object server on segment requests, which could
    cause segment requests to fail with a 304 or 412 response. Now only
    certain headers are copied to the segment requests, and that doesn't
    include the conditional ones, so they can't goof up the segment
    retrieval.
    
    Note that I moved SegmentedIterable to swift.common.request_helpers
    because it sprouted a transitive dependency on swob, and leaving it in
    utils caused a circular import.
    
    Bonus fix: unified the handling of DiskFileQuarantined and
    DiskFileNotFound in object server GET and HEAD. Now in either case, a
    412 will be returned if the client said "If-Match: *". If not, the
    response is a 404, just like before.
    
    Closes-Bug: 1279076
    Closes-Bug: 1280022
    Closes-Bug: 1280028
    
    Change-Id: Id2ee78346244d516b980202e990aa38ce6812de5

commit a94be9443d639dc0ac8498d8a35fed2832379ee7
Author: Shane Wang <shane.wang@intel.com>
Date:   Fri Feb 7 16:06:12 2014 +0800

Fix misspellings in swift
    
    Fix misspellings detected by:
    * pip install misspellings
    * git ls-files | grep -v locale | misspellings -f -
    
    Change-Id: I6594fc4ca5ae10bd30eac8a2f2493a376adcadee
    Closes-Bug: #1257295

commit 36adcb6c66bf0c0093ead1080078ca8154b0d158
Author: anc <alistair.coles@hp.com>
Date:   Tue Feb 18 14:07:37 2014 +0000

Fix invalid account acl generating 500 response.
    
    Sending an account POST with an X-Account-Access-Control
    header value that is valid json but not a valid ACL was
    causing a 500 Internal Error if the value did not parse
    to a dict due to an exception being raised in tempauth.py.
    
    This patch modifies acl.py to check that the header value
    is both json and parses to a dict. The existing
    tests are extended to cover these invalid header values.
    
    This patch also enables json encoded dicts with whitespace
    (e.g. '{ }') to be accepted as a value for
    X-Account-Access-Control in the same way that '{}' is.
    These previously resulted in a 400 response.
    
    Closes-bug: 1281626
    
    Change-Id: Ia06ba9c9d16f749f801a8158e73d3898c4a42888

commit 2e220ca9121e3f1b7359de0259a324e0533fa55e
Author: Donagh McCabe <donagh.mccabe@hp.com>
Date:   Tue Feb 18 09:41:25 2014 +0000

Improve StaticWeb 404 on web-listings/index
    
    A common scenario is for users to make a container public via an ACL.
    They can then use a browser to display the objects in the container.
    Next, they enable StaticWeb with something such as
    "X-Container-Meta-Web-Index:index.html" -- but then get confused
    because they get 404 Not Found (when index.html does not exist).
    
    For someone who understands what they are doing, this makes sense.
    However, we've had several customer escalations because of this.
    Usually, they are just playing with our GUI and have no intention
    of using StaticWeb for real -- the 404 looks like something is
    broken; not the correct response.
    
    The solution is to provide a better error message. This message is only
    shown when StaticWeb is refusing to give a listing -- other 404 situations
    are not affected. Also, a custom 404 error page is not affected.
    
    Change-Id: I3ba8c48e0ce148d8e91a1e0dc16a0d37a692a24e

commit b24a7dd4cbac8bd019af4e73def1712ba654ad63
Author: Christian Schwede <christian.schwede@enovance.com>
Date:   Mon Feb 17 10:31:20 2014 +0000

Allow log disable in account/container server
    
    Logging can be disabled in the object server, but not in the
    account and container server though both sample configs already
    include that setting. This patch allows to disable logging for
    account and container server as well.
    
    Functionality is ensured by some additional tests.
    
    Closes-Bug: 1280954
    Change-Id: Ia4e87911863089c3bea093d9f181ff29e9e963eb

commit 5f503916f15bbbeaba67d9f1631b983abe144129
Author: Luis de Bethencourt <luis@debethencourt.com>
Date:   Thu Feb 13 16:45:07 2014 -0500

small typo in cli/recon.py
    
    Change "Can't check for async's on non object servers." to "Can't check
    asyncs on non object servers."
    
    Change-Id: I04f09187da73bb3709d11d8c6f3bb1901005a0cc

commit a519579c606725d7a63aac2ca9f8a1e4520b77b1
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Wed Feb 12 15:48:29 2014 -0800

Small efficiency improvement for SLO range GET
    
    If a client sends a GET request with a Range header for a SLO
    manifest, SLO would notice this and perform a second GET request, sans
    Range header, to retrieve the manifest body. Most of the time, this is
    the sane thing to do: a subset of a SLO manifest is pretty much
    useless. However, if the client has requested the first N bytes of the
    large object, and the SLO manifest has N or fewer bytes, then the
    response will actually have the whole manifest in it, so SLO doesn't
    need to make a second request. (It's not wrong, just inefficient.)
    
    Fortunately, ranged GET responses come with a Content-Range header
    that tells us first byte, last byte, and total content length. Now SLO
    detects whether or not a ranged GET response contains the full
    manifest, and if so, just uses the existing response instead of making
    another.
    
    Change-Id: I63b991e08a1165eb3e88bd0620a1e64ef056fec6

commit ab98fe3d74f12623822395dc288d223f6995a245
Author: John Dickinson <me@not.mn>
Date:   Tue Feb 4 15:17:11 2014 -0800

Added docs about the swift_source log field
    
    Change-Id: I934230a006c856de844d8580f3b57f38a9845844

commit defccac7aff5f234fcac6ad5ccef0f855add416c
Author: Christian Schwede <christian.schwede@enovance.com>
Date:   Sun Dec 29 16:41:58 2013 +0000

Limit logged headers in proxy_logging middleware
    
    Currently all headers are logged if access_log_headers are enabled in
    proxy_logging middleware. By adding the option access_log_headers_only
    it is now possible to limit the logged headers to the given header values.
    
    DocImpact
    Change-Id: I0a1e40567c9eddc9bb00dd00373dc6eeb33d347c

commit 946984cb84414c84fe38d4b94b73174960e023b8
Author: Fabien Boucher <fabien.boucher@enovance.com>
Date:   Tue Dec 10 18:31:55 2013 +0100

Handle COPY verb in container quota middleware
    
    The patch fix the ability to bypass the account
    quota using the COPY verb. The quotas for destination
    container are now checked before the copy.
    
    Change-Id: I9f409ea6c0893f36972aa1fabe31fb143e151ab5
    Fixes: bug #1259565

commit 053cd092b1666455907427732f455bf28132a316
Author: Chmouel Boudjnah <chmouel@enovance.com>
Date:   Mon Aug 12 13:04:40 2013 +0200

Allow multiple storage_domain in cname_lookup.
    
    Implements: blueprint multiple-domains-in-domain-remap
    Change-Id: Ia647f6777717e9ff4955ea66b29f072ac03d2785

commit 0ff39c14c3b66e664491a54ac165ef3ed159bbb0
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Thu Dec 19 10:53:04 2013 -0800

Fix fd leak from get_logger() in python 2.6
    
    Calling get_logger({}) instantiates a logging.handlers.SyslogHandler,
    which opens and keeps a socket around (either /dev/log or UDP or
    whatever; not important).
    
    Under Python 2.6, all logging handlers instantiated anywhere at all
    will live for the entire lifetime of the program; they get stored in
    logging._handlerList and logging._handlers. Python 2.7 is very
    similar, but uses weakrefs instead of strong references in those
    module-level variables, so logging handlers can actually get cleaned
    up prior to program exit.
    
    The net effect is that any program that calls get_logger() more than a
    fixed number of times will leak file descriptors under Python 2.6.
    
    This commit throws encapsulation out the window and, under 2.6 only,
    replaces strong references with weakrefs in logging._handlerList and
    logging._handlers, thus avoiding the leak.
    
    Change-Id: I5dc0d1619c5a4500f892b898afd9e3668ec0ee7c

Report a bug

This report contains Public information

Everyone can see this information.

You are

Subscribing...

Edit bug mail

Other bug subscribers

Remote bug watches

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