Server side copy with Single Ranged read not working with Erasure Coded Data

Bug #1467677 reported by Douglas Soltesz
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Critical
Kota Tsuyuzaki

Bug Description

Using Server Side Copy on an erasure coded object in conjunction with a single ranged read results in an error. Both Server Side Copy and Single Ranged Reads against erasure coded data work individually.

Example:
curl -i -X PUT -H "$token" $surl/ECData/mybook --data-binary 'It was the best of times, it was the worst of times'

curl -i -X COPY -H "$token" -H "Range: bytes=7-15" -H "Destination: ECData/mybook-short3" $surl/ECData/mybook
HTTP/1.1 499 Client Disconnect
Content-Length: 89
X-Copied-From-Last-Modified: Mon, 22 Jun 2015 20:12:22 GMT
X-Copied-From: RReads/mybook
X-Copied-From-Account: AUTH_swiftstack
Content-Type: text/html; charset=UTF-8
X-Trans-Id: txfad79ed2751c4c7dac722-0055887b6a
Date: Mon, 22 Jun 2015 21:17:31 GMT

<html><h1>Client Disconnect</h1><p>The client was disconnected during request.</p></html>

CVE References

Revision history for this message
John Dickinson (notmyname) wrote :

Seeing as this is a difference in behavior from replicated storage, this should be fixed as quickly as possible.

Changed in swift:
status: New → Confirmed
importance: Undecided → High
tags: added: ec
Minwoo Bae (minwoob)
Changed in swift:
assignee: nobody → Minwoo Bae (minwoob)
Revision history for this message
Kota Tsuyuzaki (tsuyuzaki-kota) wrote :

Hi, Minwoo, have you started to work on this?

I was taking a time to look at this and found the reason for "Client disconnected" that comes from the status and content_length difference between replicated and EC at source response. And now I noticed you have a ticket for this.

If you've already have an idea to fix this, I'm willing to stop my digging this more deeply and just to wait your commit so that please feel free to let me know your status on this work?

Revision history for this message
Samuel Merritt (torgomatic) wrote :

See also https://review.openstack.org/156923; I think that has a good chance to fix this as well.

Revision history for this message
Minwoo Bae (minwoob) wrote :

Kota, I've been digging to find the core issue as well. Feel free to chime in. I've been going through the code related to the COPY request, as well as through Sam's reference above. It looks like this is a high priority bug, and may not be able to wait until the middleware patches land.

Revision history for this message
Kota Tsuyuzaki (tsuyuzaki-kota) wrote :

@Sam

Nice, suggestion. When I just was trying to make sure the copy middleware, this issue didn't occur so agreed it's good chance to land these middleware as also fix to this. I'll try to figure out these middleware behaviors.

@Minwoo
Thanks for working and I'd continue to look at this. As Sam suggested, we might go a good way by landing these patches above instead of making fix patches in proxy apps. However, I agree also what you said, this is a high priority bug so that i'd try to find a better way like merging just fixes from these middleware partially.

Revision history for this message
Kota Tsuyuzaki (tsuyuzaki-kota) wrote :
Changed in swift:
assignee: Minwoo Bae (minwoob) → Kota Tsuyuzaki (tsuyuzaki-kota)
status: Confirmed → In Progress
importance: High → Critical
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/201055
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=99d052772a9585e0befdfd292fd03aefde77180a
Submitter: Jenkins
Branch: master

commit 99d052772a9585e0befdfd292fd03aefde77180a
Author: Kota Tsuyuzaki <email address hidden>
Date: Mon Jul 13 01:12:43 2015 -0700

    Fix 499 client disconnected on COPY EC object

    Currently, a COPY request for an EC object might go to fail as 499 Client
    disconnected because of the difference between destination request content
    length and actual transferred bytes.

    That is because the conditional response status and content length for
    an EC object range GET is handled at calling the response instance on
    proxy server. Therefore the calling response instance (resp()) will change
    the conditional status from 200 (HTTP_OK) to 206 (PartialContent) and will
    change the content length for the range GET.

    In EC case, sometimes Swift needs whole stored contents to decode a segment.
    It will make 200 HTTP OK response from object-server and proxy-server
    will unfortunately set whole content length to the destination content
    length and it makes the bug 1467677.

    This patch introduces a new method "fix_conditional_response" for
    swift.common.swob.Response that calling _response_iter() and cached the
    iter in the Response instance. By calling it, Swift can set correct condtional
    response any time after setting whole content_length to the response
    instance like EC case.

    Change-Id: If85826243f955d2f03c6ad395215c73daab509b1
    Closes-Bug: #1467677

Changed in swift:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/crypto)

Fix proposed to branch: feature/crypto
Review: https://review.openstack.org/205579

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

Reviewed: https://review.openstack.org/205579
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=8ab46b64365b8eab80680f2562f81e8adb3032a3
Submitter: Jenkins
Branch: feature/crypto

commit 89f705e8aab144092d40a13fc4ef19ffef5f3eba
Author: OpenStack Proposal Bot <email address hidden>
Date: Thu Jul 23 06:11:27 2015 +0000

    Imported Translations from Transifex

    For more information about this automatic import see:
    https://wiki.openstack.org/wiki/Translations/Infrastructure

    Change-Id: I94cf347564cb33977f33b1e64259bcb39a8cf809

commit a65e9db8752793ec37b594dc9eca5066171825db
Author: Christian Schwede <email address hidden>
Date: Wed Jul 22 10:43:17 2015 +0000

    Removing commented out code in test/unit/account/test_backend.py

    Noticed this while reviewing another change. Looks like the test itself already
    ensures correct functionality of the reclaim() method in AccountBroker without
    the commented code, thus removing this stale code.

    Change-Id: I6a26a7591adef9fd794ca68a4e9c493d1127f93c

commit 99d052772a9585e0befdfd292fd03aefde77180a
Author: Kota Tsuyuzaki <email address hidden>
Date: Mon Jul 13 01:12:43 2015 -0700

    Fix 499 client disconnected on COPY EC object

    Currently, a COPY request for an EC object might go to fail as 499 Client
    disconnected because of the difference between destination request content
    length and actual transferred bytes.

    That is because the conditional response status and content length for
    an EC object range GET is handled at calling the response instance on
    proxy server. Therefore the calling response instance (resp()) will change
    the conditional status from 200 (HTTP_OK) to 206 (PartialContent) and will
    change the content length for the range GET.

    In EC case, sometimes Swift needs whole stored contents to decode a segment.
    It will make 200 HTTP OK response from object-server and proxy-server
    will unfortunately set whole content length to the destination content
    length and it makes the bug 1467677.

    This patch introduces a new method "fix_conditional_response" for
    swift.common.swob.Response that calling _response_iter() and cached the
    iter in the Response instance. By calling it, Swift can set correct condtional
    response any time after setting whole content_length to the response
    instance like EC case.

    Change-Id: If85826243f955d2f03c6ad395215c73daab509b1
    Closes-Bug: #1467677

commit 62ed4f81ef80440550633eaaaa962a4f9383c2d3
Author: Timur Alperovich <email address hidden>
Date: Tue Jul 14 16:56:44 2015 -0700

    Add two functional tests for delimiter.

    The first test verifies that a delimiter will trim entries beyond the
    first matching instance of delimiter (after the given matching prefix,
    if any) and squash duplicates. So, when setting the delimiter
    to "-", given blobs "test", "test-foo" and "test-bar-baz", we expect
    only "test" (no matching delim) and "test-" (trim all characters after
    the first "-", and squash duplicates).

    The second test verifies that when a prefix is provid...

tags: added: in-feature-crypto
Thierry Carrez (ttx)
Changed in swift:
milestone: none → 2.4.0
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/hummingbird)

Fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/221410

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

Reviewed: https://review.openstack.org/221410
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=eb8f1f83f1cfc63d8452bc30096fd1c145781527
Submitter: Jenkins
Branch: feature/hummingbird

commit cb683d391cb66d0f52830de16760c80fd2afedf9
Author: OpenStack Proposal Bot <email address hidden>
Date: Sat Sep 5 06:17:51 2015 +0000

    Imported Translations from Transifex

    For more information about this automatic import see:
    https://wiki.openstack.org/wiki/Translations/Infrastructure

    Change-Id: I2d92b8e34a665fb0bb4c048cfb0c59de295dfce6

commit e4542455c8a07b7981c247df8b737816062c1655
Author: Emett Speer <email address hidden>
Date: Wed Sep 2 17:18:03 2015 -0700

    [Labs] Update links to Cloud Admin Guide

    Update links to the Cloud Admin Guide after the
    RST conversion of that book altered URLs.

    Change-Id: I899f8938498b744e62887968a65e58c00ef27f1b

commit 58fcc07523978306cd3889ada73af5d9e664cf59
Author: Christian Schwede <email address hidden>
Date: Wed Sep 2 10:52:34 2015 +0000

    Test if container_sweep is executed on unmounted devices

    This change ensures that container_sweep is not run if a device is not mounted
    and mount_check is set to True.

    Change-Id: I823083c8431d9e61fd426508033ec9188503957b

commit e02609c66a804845672413b06830b87395afef31
Author: Samuel Merritt <email address hidden>
Date: Tue Sep 1 15:19:50 2015 -0700

    Preserve traceback in swift-dispersion-report

    Commit c690bcb fixed a bug in the dispersion report, but changed this
    from a bare "raise" to "raise err", which loses the traceback. Not a
    big deal, but worth putting back IMO.

    Change-Id: Id5b72153a4b8df8e3faaf1fa3fb2040e28ba85cc

commit d06d4ad0fd2dfe69da8008e729651264522c6c06
Author: Minwoo Bae <email address hidden>
Date: Tue Sep 1 15:08:44 2015 -0500

    Included reference in swift.obj.diskfile to enumerate the string
    used for data file paths.

    Change-Id: Ie22caa678bc00dfc43fabec7efbbb9f34490f1b5

commit 615c7a204b9386e05c5bab658bfe96766ad1e680
Author: Brian Cline <email address hidden>
Date: Tue Sep 1 10:51:20 2015 -0500

    Adds useful dispersion info from changelog

    Change-Id: I1a45088fc32620b02ff9a754b02ec1eb75a59d6e

commit 3b8755098a1786c5447abf158bd686293a82977c
Author: janonymous <email address hidden>
Date: Sun Aug 2 21:29:13 2015 +0530

    Replace a / b with a // b to use integer division where needed

    Change-Id: I72c81faa62786e140b0de00e3a04934bf1b5adbd

commit 524c89b7eeff037b8a6b421888771e15f98c2da2
Author: John Dickinson <email address hidden>
Date: Fri Aug 21 13:39:41 2015 -0700

    Updated CHANGELOG, AUTHORS, and .mailmap for 2.4.0 release.

    Change-Id: Ic6301146b839c9921bb85c4f4c1e585c9ab66661

commit 05de1305a903ee4ce9c8c50fde53c552d5b90d51
Author: Clay Gerrard <email address hidden>
Date: Thu Aug 27 18:35:09 2015 -0700

    Make ssync_sender send valid chunked requests

    The connect method of ssync_sender tells the remote connection that it's
    going to send a valid HTTP chunked request, but if the remote end needs
    to respond with an error of any kind sender th...

tags: added: in-feature-hummingbird
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.