Skip to content

Fix ts_http:split_body for non-chunked responses#302

Merged
nniclausse merged 1 commit into
processone:developfrom
tisba:fix-http-split-body
May 23, 2018
Merged

Fix ts_http:split_body for non-chunked responses#302
nniclausse merged 1 commit into
processone:developfrom
tisba:fix-http-split-body

Conversation

@tisba

@tisba tisba commented Apr 30, 2018

Copy link
Copy Markdown
Collaborator

We observed this error a lot for a recent test:

** Reason for termination = 
** {data_error,[{zlib,inflateEnd_nif,[#Ref<0.412400069.282722305.201902>],[]},
                {zlib,gunzip,1,[]},
                {ts_http,decode_buffer,2,
                         [{file,"src/tsung/ts_http.erl"},{line,72}]},
                {ts_client,handle_data_msg,2,
                           [{file,"src/tsung/ts_client.erl"},{line,1127}]},
                {ts_client,handle_info2,3,
                           [{file,"src/tsung/ts_client.erl"},{line,229}]},
                {gen_fsm,handle_msg,8,[{file,"gen_fsm.erl"},{line,483}]},
                {proc_lib,init_p_do_apply,3,
                          [{file,"proc_lib.erl"},{line,247}]}]}

The request was made with Accept-Encoding: gzip and the server answered with an gzip encoded response. BUT the server did send the response without Transfer-Encoding: chunked. The previous implementation of ts_http:split_body/1 did append a newline to the body.

This PR changes two things:

  1. simplified & fixed the regular expression used to perform the split: The body match needs to be greedy in order to collect trailing newlines if present
  2. do no longer append \n to the extracted body. The response body does not need to be terminated by \r\n (unless the response is chunked, which is never the case when ts_http:split_body/1 is used)

We first wondered, why this hasn't been a big problem in our many thousand test executions because zlib:gunzip/1 will always fail if you append <<10>>. The reason why this did not happen all the time is, that it is very uncommon to have a not-chunked but gzipped response. In case the response is chunked ts_http:decode_chunk/1 is used to perform the split of headers and body which does not suffer from the same problem.

@tisba

tisba commented May 23, 2018

Copy link
Copy Markdown
Collaborator Author

Sorry for pinging, @nniclausse. But could you also have a quick look on this? Although this is a rare case, we've tested this quite a bit against different scenarios.

I'd really like to narrow down the amount of patches I have to maintain :)

@nniclausse nniclausse merged commit 186791d into processone:develop May 23, 2018
@nniclausse

Copy link
Copy Markdown
Contributor

Merged. Don't hesitate to ping me !

@tisba tisba deleted the fix-http-split-body branch May 29, 2018 07:57
@WGH-

WGH- commented Jul 15, 2020

Copy link
Copy Markdown

Any chance of having of this fix released?

@WGH-

WGH- commented Jul 16, 2020

Copy link
Copy Markdown

For example, Go default net/http server tend to return non-chunked responses if the response is short enough to be buffered, even if Content-Length is not explicitly specified: https://github.com/golang/go/blob/c5d7f2f1cbaca8938a31a022058b1a3300817e33/src/net/http/server.go#L338-L341 . If one uses Go as a reverse-proxy, and the backend is compressing responses, this happens all the time.

@nniclausse nniclausse added this to the 1.8.0 milestone Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants