>>>>This restriction is per transaction, I couldn't think of a case where you
>>>>want to call the same continuation in the same hook for the same
>>>>transaction multiple times.
Hmm..IIRC, collapsed_forwarding relies on calling the same continuation in the
same hook for the same Txn multiple times (this is a direct consequence of
redirect follow handling in the core, reusing the current Txn and not creating
a separate Txn - which was something that we (I think it was Peach?) discussed
changing in the past).
Anyway, like Vijay replied, as long as Meera's solution doesn't break calling
the same hook multiple times on the same Cont/Txn, I agree it seems fine :)
Thanks,
Sudheer
From: Leif Hedstrom <zw...@apache.org>
To: dev@trafficserver.apache.org; Sudheer Vinukonda
<sudheervinuko...@yahoo.com.INVALID>
Sent: Tuesday, November 29, 2016 2:33 PM
Subject: Re: Multiple times gzip issue and TSHttpTxnHookAdd API
> On Nov 29, 2016, at 1:53 PM, Sudheer Vinukonda
> <sudheervinuko...@yahoo.com.INVALID> wrote:
>
> I think there are some plugins that do depend on a hook getting called
> multiple times (example, redirect response based plugins like escalate,
> collapsed_forwarding etc).
This restriction is per transaction, I couldn't think of a case where you want
to call the same continuation in the same hook for the same transaction
multiple times.
Basically, what Meera is suggesting is that we consider TSHttpTxnHookAdd() to
be idempotent. There’s a small caveat here though: it’s only considering each
continuation (basically pointer) as the idempotent reference. To put semantics
into this, such as saying “Only allow one gzip continuation”, more information
is needed in the core, something that Alan has promised to add any minute now.
> To solve the issue of multiple gzips, would it be simpler to store in the
> continuation the info that gzip has already been completed? If not, may be
> Txn arg or even an @header ?
Right, that’s the path we headed down first. But it turns out, many plugins
could / would have this issue because of how the HttpSM works. Remember, the
issue is that when a plugin like escalate.so (or via records.config) restarts
the HttpSM, it does *not* reset the list of continuations to call.
So, it seems better to solve this once and for all, instead of having to fix
every possible plugin that would run into this if e.g. the escalate plugin is
used.
Cheers,
— Leif
>
> Thanks,
>
> Sudheer
>
>> 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.
>>
>> 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.
>
