You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Meera Mosale Nataraja <me...@gmail.com> on 2016/11/29 19:38:26 UTC
Multiple times gzip issue and TSHttpTxnHookAdd API
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.
Re: Multiple times gzip issue and TSHttpTxnHookAdd API
Posted by vijay mamidi <vi...@gmail.com>.
I think the patch is going to restrict plugin registering to the same hook
multiple times but i don't think it will restrict number of times a hook is
called.
-Vijay
On Tue, Nov 29, 2016 at 12:53 PM, Sudheer Vinukonda <
sudheervinukonda@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).
>
> 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 ?
>
> Thanks,
>
> Sudheer
>
> > On Nov 29, 2016, at 11:38 AM, Meera Mosale Nataraja <me...@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.
>
>
Re: Multiple times gzip issue and TSHttpTxnHookAdd API
Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
>>>>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 <su...@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 <su...@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 <me...@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.
>
Re: Multiple times gzip issue and TSHttpTxnHookAdd API
Posted by Leif Hedstrom <zw...@apache.org>.
> On Nov 29, 2016, at 1:53 PM, Sudheer Vinukonda <su...@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 <me...@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.
>
Re: Multiple times gzip issue and TSHttpTxnHookAdd API
Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
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).
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 ?
Thanks,
Sudheer
> On Nov 29, 2016, at 11:38 AM, Meera Mosale Nataraja <me...@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.
Re: Multiple times gzip issue and TSHttpTxnHookAdd API
Posted by Meera Mosale Nataraja <me...@gmail.com>.
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 <jp...@apache.org> wrote:
> >
> >>
> >> On Nov 29, 2016, at 11:38 AM, Meera Mosale Nataraja <me...@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.
>
>
Re: Multiple times gzip issue and TSHttpTxnHookAdd API
Posted by Leif Hedstrom <zw...@apache.org>.
> On Nov 29, 2016, at 7:57 PM, James Peach <jp...@apache.org> wrote:
>
>>
>> On Nov 29, 2016, at 11:38 AM, Meera Mosale Nataraja <me...@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.
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.
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.
Re: Multiple times gzip issue and TSHttpTxnHookAdd API
Posted by James Peach <jp...@apache.org>.
> On Nov 29, 2016, at 11:38 AM, Meera Mosale Nataraja <me...@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.
I don’t think this would be enough for gzip either, since it creates a nee transform continuation each time.
>
> 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.