[ https://issues.apache.org/jira/browse/TS-3740?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14695916#comment-14695916 ]
Gancho Tenev edited comment on TS-3740 at 8/13/15 8:50 PM: ----------------------------------------------------------- Yes, just by reading around the 2 patches (didn't have a chance to experiment with TS-3137 patch) it seems that they are meant to accomplish the same thing - enable set-redirect operation when not called from the remap plugin. There is an important difference in the replacing of "PATH" and in "QSA mode" (appending query parameters to) when forming the "Location" header. TS-3740 patch always uses client request URI when replacing those variables to form the "Location" header regardless of which hook condition matches while TS-3137 patch uses the corresponding URI at each particular hook (which is consistent with the way set-destination is implemented). I don't have visibility of all header-rewrite use-cases but it seems that although at the time it looked more reasonable to always use the client request URI to form "Location" header (it is always available regardless of which hook condition matches, it fitted the above origin time-out use-case well and seemed a more straightforward way to configure the redirects), it may be more reasonable to do it in TS-3137 way (which looks also more consistent with set-destination operation implementation as well). Any ideas and opinions are appreciated! was (Author: gancho): Yes, just by reading around the 2 patches (didn't have a chance to experiment with TS-3137 patch) it seems that they are meant to accomplish the same thing - enable set-redirect operation when not called from the remap plugin. There is an important difference in the replacing of "%{PATH}" and in "QSA mode" (appending query parameters to) when forming the "Location" header. TS-3740 patch always uses client request URI when replacing those variables to form the "Location" header regardless of which hook condition matches while TS-3137 patch uses the corresponding URI at each particular hook (which is consistent with the way set-destination is implemented). I don't have visibility of all header-rewrite use-cases but it seems that although at the time it looked more reasonable to always use the client request URI to form "Location" header (it is always available regardless of which hook condition matches, it fitted the above origin time-out use-case well and seemed a more straightforward way to configure the redirects), it may be more reasonable to do it in TS-3137 way (which looks also more consistent with set-destination operation implementation as well). Any ideas and opinions are appreciated! > header_rewrite plugin: set-redirect doesn't work with SEND_RESPONSE_HDR_HOOK > ---------------------------------------------------------------------------- > > Key: TS-3740 > URL: https://issues.apache.org/jira/browse/TS-3740 > Project: Traffic Server > Issue Type: Bug > Components: Plugins > Reporter: Gancho Tenev > Assignee: Gancho Tenev > Fix For: 6.1.0 > > > DESCRIPTION: > ATS header_rewrite plugin set-redirect operation doesn't work with > SEND_RESPONSE_HDR_HOOK. Please see the debugging notes below for more info. > HOW TO REPRODUCE: > Here is a sample plugin configuration files that reproduce the problem > $ cat /opt/ats/etc/trafficserver/remap.config > map http://p1 http://h1:8001 \ > @plugin=header_rewrite.so > @pparam=/opt/ats/etc/trafficserver/header_rewrite.config > $ cat /opt/ats/etc/trafficserver/header_rewrite.config > cond %{SEND_RESPONSE_HDR_HOOK} > cond %{STATUS} =502 > set-redirect 302 http://p0/%{PATH} [QSA] > DEBUGGING NOTES: > Both conditions in the header_rewrite.config are evaluated correctly but > set-redirect has no effect and the response to the UA is not modified as > expected. After some debugging it turned out that if the set-redirect > (OperatorSetDestination::exec) is not called from the remap plugin it has no > effect. The header_rewrite plugin creates a continuation to be called from > SEND_RESPONSE_HDR_HOOK (TSHttpHookAdd()). OperatorSetDestination::exec > doesn't have code to handle the case when the set-redirect operation is _not_ > called directly from the remap plugin (TSRemapDoRemap()). -- This message was sent by Atlassian JIRA (v6.3.4#6332)