[jira] [Comment Edited] (TS-4299) FetchSM / H2 streams can hang if e.g. a plugin adds a Connection: close header

["Leif Hedstrom (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TS-4299?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15213064#comment-15213064 ] 

Leif Hedstrom edited comment on TS-4299 at 3/26/16 2:49 PM:
------------------------------------------------------------

This is easily reproduced (in my environment at least) with a header_rewrite rule like:

{code}
cond %{SEND_RESPONSE_HDR_HOOK}
  set-header Connection "close"
{code}



was (Author: zwoop):
This is easily reproduced (in my environment at least) with a header_rewrite rule like:

{code}
cond %{SEND_RESPONSE_HDR_HOOK}
  set-header Connection "close"
{close}


> FetchSM / H2 streams can hang if e.g. a plugin adds a Connection: close header
> ------------------------------------------------------------------------------
>
>                 Key: TS-4299
>                 URL: https://issues.apache.org/jira/browse/TS-4299
>             Project: Traffic Server
>          Issue Type: Bug
>          Components: Core, HTTP/2
>            Reporter: Leif Hedstrom
>             Fix For: 6.2.0
>
>
> We use header_rewrite to inject Connection: close under certain conditions (to prevent KA connections from lingering forever during e.g. server maintenance). However, this interacts poorly with the FetchSM, and therefore, poorly with H2. Basically, the connection "hangs" because the FetchSM thinks there's more body data (when there really isn't).
> The issue comes from this code:
> {code}
> bool
> FetchSM::has_body()
> {
>   int status_code;
>   HTTPHdr *hdr;
>   if (!header_done)
>     return false;
>   if (is_method_head)
>     return false;
>   //
>   // The following code comply with HTTP/1.1:
>   // http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4
>   //
>   hdr = &client_response_hdr;
>   status_code = hdr->status_get();
>   if (status_code < 200 || status_code == 204 || status_code == 304)
>     return false;
>   if (check_chunked())
>     return true;
>   if (check_connection_close())
>     return true;
>   resp_content_length = hdr->value_get_int64(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH);
>   if (!resp_content_length)
>     return false;
>   return true;
> }
> {code}
> The issue is that it returns "true" if there is a Connection: close header. I'm not 100% sure what the best solution here, simply removing this doesn't seem right since it's obviously there to deal with some cases of FetchSM which are not H2. A couple of unscientific options are
> 1) Make sure the H2 code assures that the Connection header is removed in all cases. I know it deals with it in a few cases, except in this case we add the Connection: close in the response header object, which therefore enters into the FetchSM as well.
> 2) Have additional state / information in the FetchSM, indicating if "connection" has semantics or not for this particular FetchSM. E.g. fetch_sm->set_connection_aware(false);



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)