connection closed before XML bulk delete data can be transmitted

Bug #1819252 reported by david
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Undecided
Unassigned

Bug Description

It appears that since the following code change:
 https://review.openstack.org/#/c/510715/
a bulk delete that wants XML output emitted will fail when used with HTTP "Expect: 100-continue". The XML header is emitted and then the HTTP connection is closed by Swift before any body data can be sent.

The application in question is based on libcurl, which is making use of the "Expect: 100-continue" header based on it's own hueristics.

Example of this occurring against a Swift 2.17 production proxy server:

[dcc@mbp ~]$ curl -X DELETE https://london01.storage.sohonet.com/v1/AUTH_bc8242fea43146a7b8cee34a40f328e0\?bulk-delete -H "X-Auth-Token: $TOKEN" -H "Accept: application/xml" -H "Expect: 100-continue" -d "foo" -H 'Content-Type: text/plain'
<?xml version="1.0" encoding="UTF-8"?>
curl: (18) transfer closed with outstanding read data remaining

Without the Expect header:

[dcc@mbp ~]$ curl -X DELETE https://london01.storage.sohonet.com/v1/AUTH_bc8242fea43146a7b8cee34a40f328e0\?bulk-delete -H "X-Auth-Token: $TOKEN" -H "Accept: application/xml" -d "foo" -H 'Content-Type: text/plain'
<?xml version="1.0" encoding="UTF-8"?>
<delete>
<number_deleted>0</number_deleted>
<number_not_found>1</number_not_found>
<response_body></response_body>
<response_status>200 OK</response_status>
<errors>
</errors>
</delete>

Checking the code change @ https://review.openstack.org/#/c/510715/1/swift/common/middleware/bulk.py@387, if I move the "req.environ['eventlet.minimum_write_chunk_size'] = 0" lines on a staging proxy-server back to where they originally were, I get the correct behaviour:

[dcc@mbp ~]$ curl -X DELETE https://london-staging.storage.sohonet.com/v1/AUTH_bc8242fea43146a7b8cee34a40f328e0\?bulk-delete -H "X-Auth-Token: $TOKEN" -H "Accept: application/xml" -H "Expect: 100-continue" -d "foo" -H 'Content-Type: text/plain'
<?xml version="1.0" encoding="UTF-8"?>
<delete>
<number_deleted>0</number_deleted>
<number_not_found>1</number_not_found>
<response_body></response_body>
<response_status>200 OK</response_status>
<errors>
</errors>
</delete>

Swift 2.17
haproxy 1.6 ( I have bypassed the proxy and confirm the issue still occcurs)

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

I have duplicated this in my saio

Changed in swift:
status: New → Confirmed
Revision history for this message
John Dickinson (notmyname) wrote :

but I'm getting a slightly different error. Here's the tcpdump of it...

DELETE /v1/AUTH_test/?bulk-delete HTTP/1.1
Host: saio:8080
User-Agent: curl/7.54.0
X-Auth-Token: AUTH_tk130a1e256b1444639937cbaa5cfd72bb
Accept: application/xml
Expect: 100-continue
content-type: text/plain
Content-Length: 3

HTTP/1.1 200 OK
Content-Type: application/xml
X-Trans-Id: tx7d73383083f5441586cd2-005c8bf74c
X-Openstack-Request-Id: tx7d73383083f5441586cd2-005c8bf74c
Date: Fri, 15 Mar 2019 19:04:44 GMT
Transfer-Encoding: chunked

27
<?xml version="1.0" encoding="UTF-8"?>

HTTP/1.1 100 Continue

d7
<delete>
<number_deleted>0</number_deleted>
<number_not_found>0</number_not_found>
<response_body>Invalid bulk delete.</response_body>
<response_status>400 Bad Request</response_status>
<errors>
</errors>
</delete>

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

from the client-side:

$ curl -i -H "X-Auth-Token: AUTH_tk130a1e256b1444639937cbaa5cfd72bb" "http://saio:8080/v1/AUTH_test/?bulk-delete" -XDELETE -H "Accept: application/xml" -H "Expect: 100-continue" -d "c/o" -H "content-type: text/plain"
HTTP/1.1 200 OK
Content-Type: application/xml
X-Trans-Id: tx7d73383083f5441586cd2-005c8bf74c
X-Openstack-Request-Id: tx7d73383083f5441586cd2-005c8bf74c
Date: Fri, 15 Mar 2019 19:04:44 GMT
Transfer-Encoding: chunked

<?xml version="1.0" encoding="UTF-8"?>
curl: (56) Illegal or missing hexadecimal sequence in chunked-encoding

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

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

commit adc568c97f5b30d9d4628eaf448f81d736ad4e51
Author: John Dickinson <email address hidden>
Date: Fri Mar 15 15:18:36 2019 -0700

    Fix bulk responses when using xml and Expect 100-continue

    When we fixed bulk response heartbeating in https://review.openstack.org/#/c/510715/,
    code review raised the issue of moving the xml header down to after the
    early-exit clauses. At the time, it didn't seem to break anything, so
    it was left in place. However, that insight was correct.

    The purpose of the earlier patch was to force eventlet to use chunked
    transfer encoding on the response in order to prevent eventlet from
    buffering the whole response, thus defeating the purpose of the
    heartbeat responses.

    Moving the first line of the body lower (ie after the early exit
    checks), allows other headers in a chunked transfer encoding response
    to be appropriately processed before sending the headers. Sending the
    xml declaration early causes it to get intermingled in the 100-continue
    protocol, thus breaking the chunked transfer encoding semantics.

    Closes-Bug: #1819252

    Change-Id: I072f4dab21cd7cdb81b9e41072eb504131411dc8

Changed in swift:
status: Confirmed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.21.0

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

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.openstack.org/648245

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

Reviewed: https://review.openstack.org/648245
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=6afc1130fd753306d64745c9bee7712182b273d3
Submitter: Zuul
Branch: feature/losf

commit 89e5927f7dd94fc28b3847944eb7dd227d516fa8
Author: Thiago da Silva <email address hidden>
Date: Tue Mar 26 10:46:02 2019 -0400

    Fix mocking time

    When running on Centos the side_effect was returning a MagicMock
    object instead of the intended int.

    Change-Id: I73713a9a96dc415073a637d85a40304021f76072

commit 50715acb1838fbde628e447e7b02545ce8469180
Author: OpenStack Release Bot <email address hidden>
Date: Mon Mar 25 17:07:54 2019 +0000

    Update master for stable/stein

    Add file to the reno documentation build to show release notes for
    stable/stein.

    Use pbr instruction to increment the minor version number
    automatically so that master versions are higher than the versions on
    stable/stein.

    Change-Id: I6109bff3227f87d914abf7bd1d76143aaf91419d
    Sem-Ver: feature

commit 179fa7ccd4d6faeacc989715887b69f9422a17b2
Author: John Dickinson <email address hidden>
Date: Mon Mar 18 17:09:31 2019 -0700

    authors/changelog update for 2.21.0 release

    Change-Id: Iac51a69c71491e5a8db435aae396178a6c592c73

commit 64eec5fc93eb670e581cbb3a6dedb6a7aa501e99
Author: Tim Burke <email address hidden>
Date: Thu Mar 7 14:36:02 2019 -0800

    Fix how we UTF-8-ify func tests

    I noticed while poking at the DLO func tests that we don't actually use
    non-ascii chars when we set up the test env.

    By patching the create name function earlier (in SetUpClass) we can
    ensure we get some more interesting characters in our object names.

    Change-Id: I9480ddf74463310aeb11ad876b79527888d8c871

commit fe3a20f2e4b745bf7d81f9bda97082b593e8794a
Author: Tim Burke <email address hidden>
Date: Tue Mar 19 14:52:19 2019 -0700

    Remove uncalled function

    Change-Id: Ica67815f0ddf4b00bce1ffe183735490c7f7c0b5
    Related-Change: I5629de9f2e9b2331ed3f455d253efc69d030df72

commit adc568c97f5b30d9d4628eaf448f81d736ad4e51
Author: John Dickinson <email address hidden>
Date: Fri Mar 15 15:18:36 2019 -0700

    Fix bulk responses when using xml and Expect 100-continue

    When we fixed bulk response heartbeating in https://review.openstack.org/#/c/510715/,
    code review raised the issue of moving the xml header down to after the
    early-exit clauses. At the time, it didn't seem to break anything, so
    it was left in place. However, that insight was correct.

    The purpose of the earlier patch was to force eventlet to use chunked
    transfer encoding on the response in order to prevent eventlet from
    buffering the whole response, thus defeating the purpose of the
    heartbeat responses.

    Moving the first line of the body lower (ie after the early exit
    checks), allows other headers in a chunked transfer encoding response
    to be appropriately processed before sending the headers. Sending the
    xml declaration early causes it to get intermingled in the 100-continue
    protocol, thus breaking the chunked transfer encoding semantics.

    Closes-Bug: #1819...

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