Skip to content

Fix waiting for response body on HTTP HEAD requests with Content-Length set#334

Merged
tisba merged 1 commit into
developfrom
fix-http-head-requests-with-content-length
Sep 21, 2018
Merged

Fix waiting for response body on HTTP HEAD requests with Content-Length set#334
tisba merged 1 commit into
developfrom
fix-http-head-requests-with-content-length

Conversation

@tisba

@tisba tisba commented Sep 16, 2018

Copy link
Copy Markdown
Collaborator

We recently noticed, that tsung will wait on HTTP HEAD requests when Content-Length header is set to non-zero value.

According to RFC2616 section 14.13:

The Content-Length entity-header field indicates the size of the entity-body, in decimal number of OCTETs, sent to the recipient or, in the case of the HEAD method, the size of the entity-body that would have been sent had the request been a GET.

So the Content-Length header - if present - will contain the expected response size for a non-HEAD request. Actually it is recommended that Content-Length should be present (also RFC2616 section 14.13):

Applications SHOULD use this field to indicate the transfer-length of the message-body, unless this is prohibited by the rules in section 4.4.

So it is considered "normal" that a HTTP HEAD requests contains a Content-Length header.

This is the same with HTTP 1.0, according to RFC1945 Sec 10.4.

Issue

The problem in tsung was with ts_http_common:check_resp_size/4. If headers are received completely and the content length is non-zero, we always expect more data. This resulted in tsung waiting indefinitely (until server closes connection).

Solution

In case we have completely read the HTTP headers and in case of the HTTP verb is HEAD, we are done reading the response, regardless what Content-Length states. In this case we can set ack_done in #state_rcv to true.

The problem can be reproduced using

<?xml version="1.0"?>
<!DOCTYPE tsung SYSTEM "/tsung/install/share/tsung/tsung-1.0.dtd" []>
<tsung loglevel="debug" dumptraffic="true">
  <clients>
    <client host="localhost" use_controller_vm="true"/>
  </clients>
  <servers>
    <server host="testapp.loadtest.party" port="80" type="tcp"/>
  </servers>
  <load duration="300" unit="second">
    <arrivalphase phase="0" duration="300" unit="second">
      <users maxnumber="1" arrivalrate="1" unit="second" />
    </arrivalphase>
  </load>
  <sessions>
    <session name="http_head" weight="1" type="ts_http">
      <request subst="true">
        <http url="/status/200" method="HEAD" version="1.1">
        </http>
      </request>
    </session>
  </sessions>
</tsung>

Note that this host will respond with Content-Length:

$ curl --head testapp.loadtest.party
HTTP/1.1 200 OK
Content-Type: text/html
Content-Length: 112
Connection: keep-alive
Status: 200 OK
Date: Sun, 16 Sep 2018 18:08:59 GMT
X-Powered-By: Phusion Passenger 5.3.4
Server: nginx/1.14.0 + Phusion Passenger 5.3.4

@tisba

tisba commented Sep 16, 2018

Copy link
Copy Markdown
Collaborator Author

Hey @nniclausse. Could you take a quick look at this?

It took me a while to figure out what was going on here but I think the solution now is okay. But since this looks like it was so fundamentally broken, I'm a bit irritated and want to make sure I'm not overlooking something obvious here :)

@tisba tisba changed the title Fix waiting for response body for HTTP HEAD requests with Content-Length Fix waiting for response body on HTTP HEAD requests with Content-Length set Sep 17, 2018
@tisba tisba added this to the 1.8.0 milestone Sep 20, 2018
@tisba

tisba commented Sep 21, 2018

Copy link
Copy Markdown
Collaborator Author

Sorry for pining again, but could you maybe take a quick look, @nniclausse?

I've tested this quite a bit, but as I already said, this is just so strange because this was broken basically forever...

@nniclausse nniclausse left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay
Since this bug is here like forever, it means that almost nobody use HEAD requests ... :-/

@tisba

tisba commented Sep 21, 2018

Copy link
Copy Markdown
Collaborator Author

We had actually some tests that used HEAD requests, but the responses were dynamic. The application did not include a Content-Length header which is totally okay from the RFC perspective. But recent tests hitting static resources uncovered the issue, as most web servers DO include Content-Length in this case.

Anyway, thanks for taking a look! 👍

@tisba tisba merged commit 90d3c9a into develop Sep 21, 2018
@tisba tisba deleted the fix-http-head-requests-with-content-length branch September 21, 2018 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants