On Thu, Dec 1, 2016 at 10:34 AM, Leif Hedstrom <zw...@apache.org> wrote:

>
> > On Nov 29, 2016, at 7:57 PM, James Peach <jpe...@apache.org> wrote:
> >
> >>
> >> On Nov 29, 2016, at 11:38 AM, Meera Mosale Nataraja <mech...@gmail.com>
> wrote:
> >>
> >> Hello all,
> >>
> >> I'm working on TS-5024 <https://issues.apache.org/jira/browse/TS-5024>
> where
> >> the content is gzip’ed multiple times. Please find the sample output
> >> provided below which indicates multiple gzips.
> >>
> >> curl -v -o/dev/null http://proxy-test:8080/get -H "Host: proxy-test" -x
> >> localhost:8080 -H "Accept-encoding: gzip"
> >>
> >>  - About to connect() to proxy localhost port 8080 (#0)
> >>  - Trying ::1... connected
> >>  - Connected to localhost (::1) port 8080 (#0)
> >>> GET http://proxy-test:8080/get HTTP/1.1
> >>> User-Agent: curl/7.19.7 (x86_64-redhat-linux-gnu) libcurl/7.19.7
> >>  NSS/3.21 Basic ECC zlib/1.2.3 libidn/1.18 libssh2/1.4.2
> >>> Accept: */*
> >>> Proxy-Connection: Keep-Alive
> >>> Host: proxy-test
> >>> Accept-encoding: gzip
> >>>
> >>  % Total % Received % Xferd Average Speed Time Time Time Current
> >>  Dload Upload Total Spent Left Speed
> >>  0 0 0 0 0 0 0 0 -::- -::- -::- 0< HTTP/1.1 404 Not Found
> >>  < Server: ATS/7.1.0
> >>  < X-Frame-Options: SAMEORIGIN
> >>  < X-Xss-Protection: 1; mode=block
> >>  < Accept-Ranges: bytes
> >>  < X-Content-Type-Options: nosniff
> >>  < Content-Type: text/html; charset=UTF-8
> >>  < Cache-Control: max-age=300
> >>  < Expires: Mon, 31 Oct 2016 18:29:44 GMT
> >>  < Date: Mon, 31 Oct 2016 18:24:44 GMT
> >>  < Content-Encoding: gzip
> >>  < Vary: Accept-Encoding
> >>  < Content-Encoding: gzip
> >>  < Content-Encoding: gzip
> >>  < Content-Length: 4456
> >>  < Age: 0
> >>  < Proxy-Connection: keep-alive
> >>
> >>
> >> For example, if OS returns 302 and redirection is enabled, send request
> >> hook is called multiple times and the plugin then registers the
> >> TS_HTTP_READ_RESPONSE_HDR_HOOK multiple times - once for the first
> request
> >> and again for the redirected request. Hence TSHttpTxnHookAdd API adds
> the
> >> hook multiple times without checking if the hook is already present in
> the
> >> list of hooks. This will result in multiple transformations and each of
> >> them trying to gzip the content.
> >>
> >> One solution is to modify the current implementation of
> TSHttpTxnHookAdd by
> >> adding the functionality to traverse the list of hooks and append the
> hook
> >> only if it is not present in the list. Please find the changeset
> provided
> >> below.
> >
> > This isn’t sufficient when the plugin attaches a new continuation to the
> hook, eg. esi, pagespeed, cache_range_requests, remap_purge, etc.
>
> Right, this was pointe out. It covers many, but not all, cases. Fixing
> this for all cases requires noticeably more code, something that I think
> Alan is also working on.
>
> >
> > I don’t think this would be enough for gzip either, since it creates a
> nee transform continuation each time.
>
> I think it is sufficient (Meera: you tested this, right?). The issue is
> that the hook list is retained when the HttpSM restarts, and then we add
> the same continuation (same contp) multiple times.


Yes, I have tested it and it works well.

>
> The big questions here though are:
>
> 1) Can / should we treat TSHttpTxnHookAdd() as idempotent in general (by
> design)?
>
> 2) Can we “break” TSHttpTxnHookAdd() in mid-release?
>
>
> I’d argue that the answer is “yes” to both (I like to see this as a
> fundamental bug in TSHttpTxnHookAdd() :-).
>
> Now, while typing this, and thinking about it some more, another option
> *might* be that when a continuation is executed, we remove it from the Hook
> list. Meera / VJ / amc: What do you think of that solution? That *might*
> break some plugins though, unless they re-register accordingly, but it
> would solve the gzip plugin.
>

I will look into this approach as well.

>
> Cheers,
>
> — Leif
>
> >
> >>
> >> diff --git proxy/InkAPI.cc <http://inkapi.cc/> proxy/InkAPI.cc
> >> <http://inkapi.cc/>
> >> index 48697d5..8b73779 100644
> >> --- proxy/InkAPI.cc <http://inkapi.cc/>
> >> +++ proxy/InkAPI.cc <http://inkapi.cc/>
> >> @@ -4599,6 +4599,15 @@ TSHttpTxnHookAdd(TSHttpTxn txnp, TSHttpHookID id,
> >> TSCont contp)
> >> sdk_assert(sdk_sanity_check_hook_id(id) == TS_SUCCESS);
> >>
> >> HttpSM *sm = (HttpSM *)txnp;
> >> +  APIHook *hook = sm->txn_hook_get(id);
> >> +
> >> +  // Traverse list of hooks and add a particular hook only once
> >> +  while (hook != NULL) {
> >> +    if (hook->m_cont == (INKContInternal *)contp) {
> >> +      return;
> >> +    }
> >> +    hook = hook->m_link.next;
> >> +  }
> >> sm->txn_hook_append(id, (INKContInternal *)contp);
> >> }
> >>
> >> If we think a plugin would need a functionality of calling the hook
> >> multiple times we can add a new API but i can’t think of any plugin that
> >> would need such an API. Please let me know your thoughts and feedback.
> >>
> >> Meera.
>
>

Reply via email to