I would agree with this proposal. I am currently writing a plugin in which I am running into the exact issues outlined here.
On 2019/02/11 21:05:40, "Zelkowitz, Evan" <e...@comcast.com<mailto:e...@comcast.com>> wrote: > Id agree with this. Its easy enough to always get the pristine url by just > making the api call. However getting the remapped url is not as easy. So it > makes sense that the remapped becomes the default and if any plugin needs the > pristine then it can get it anyway> > > > > On 2/11/19, 1:42 PM, "Gancho Tenev" > <ga...@apache.org<mailto:ga...@apache.org>> wrote:> > > > > > > > I looked into the issues #4929, #4026, #2877 and PR #4531 which are all > related to this proposal about TSRemapRequestInfo::requestUrl (requestUrl for > short) and plugin ordering and I would like to revive the thread. (links to > issues, PRs, IRC logs referenced at the end of this email)> > > > > > I definitely support *not* having the first plugin special but would like > to propose to rewrite the url *before* running all plugins instead of *after* > which would guarantee that:> > > all plugins would get the same requestUrl (first plugin not special case)> > > all plugins would treat requestUrl the same way consistently as a > *remapped* URL which makes the first plugin logic really no different from > the rest> > > there would be a remapped URL default in case the remap rule had no > plugins OR none of the plugins modified the remapped URL.> > > > > > > > > > > > > > > The following are some thoughts and information which I hope would > justify the proposal.> > > > > > Plugin design:> > > ==============> > > > > > Based on my understanding of the relevant code and the comments in it, > the plugin execution by design allows the plugins to be ordered in a pipeline > which results in “calculating” the final remapped URL. requestUrl is meant to > be used in read/write mode as the *input* and the *output* for each > individual plugin in the pipeline.> > > > > > Each plugin from the pipeline could read requestUrl if it needs to check > what would be the eventual remapped URL if not modified and to decide to > modify it if necessary (as a whole or only parts of it by using TS API > calls). Then the next plugin in the pipeline could follow the same read > and/or modify action and so on and so forth. The pipeline progress can be > continued or aborted based on any plugin return code which also marks if the > remapped URL was modified.> > > > > > At least this was the behavior before PR #4531, except that for some > reason, not well understood, the special first plugin got the *pristine* URL > as input and the rest of the plugins got the *remapped* URL.> > > > > > > > > Even after the PR #4531 fix the first plugin still feels special?> > > =================================================================> > > > > > Now let us consider the following use case after the PR #4531 fix. > > > > > > The first plugin gets the *pristine* URL (as before instead of the > *remapped* URL) which means it will have to treat its input as *pristine* > URI. If the first plugin decides to check (read input) and modify (write > output) requestUrl then the second plugin will have to treat the input > requestUrl differently as *remapped* URL since the first plugin already > modified it and if no other plugin modifies requestUrl the content of > requestUrl will end up being the final *remapped* URL.> > > > > > Wait! The first plugin gets *pristine* URL and the rest get the > *remapped* URL? This still feels like inconsistent design since it makes the > first plugin a special case again which was something that we tried to fix > with PR #4531.> > > > > > Also PR #4531 kept the backward compatibility for the first plugin and > broke compatibility for the rest. We cannot avoid breaking compatibility but > I would prefer to break compatibility for the one (first) plugin in the chain > rather than for the rest (most) of the plugins. Based on what I have seen, > most of the configurations run with much more than a single plugin per remap. > > > > > > > Looking further into the usage, the overhead of setting a default by > overwriting URL before all plugins run on every matching transaction will be > negligible since most of the plugins don’t modify the requestUrl or if they > do, it is the case for a relatively small number of the transactions, hence > we would hit the overwrite most of the time regardless. The only exception > are plugins like regex_remap which constantly overwrite URL but even then > setting the default before regex_remap modifies the URL does not seem too > expensive.> > > > > > I have a PR #4964 which I believe should address all the aforementioned > issues. > > > > > > Please let me know what you think!> > > > > > Cheers,> > > --Gancho> > > > > > > > > Reference:> > > https://github.com/apache/trafficserver/issues/4929 - “Cachekey plugin > assumes requestUrl in TSRemapRequestInfo is remapped URL”> > > https://github.com/apache/trafficserver/issues/4026 - “Plugin ordering > changes remap URI even with empty plugin”> > > https://github.com/apache/trafficserver/issues/2877 - > “Unexpected/wrong/undocumented behavior when using regex_remap plugin”> > > https://github.com/apache/trafficserver/pull/4531 - “Rewrite url after > running all remap plugin”> > > https://github.com/apache/trafficserver/pull/4964 > <https://github.com/apache/trafficserver/pull/4964> - "Rewrite url before all > remap plugins run” (NEW)> > > https://wilderness.apache.org/channels/?f=traffic-server/2018-11-29 - IRC > discussion about “Rewrite url after running all remap plugin”> > > > > > > > > > > > > > > > On Dec 3, 2018, at 12:09 PM, Alan Carroll > <so...@oath.com.INVALID<mailto:so...@oath.com.INVALID>> wrote:> > > > > > > > This came up again on IRC. The effect of the PR is> > > > > > > > Before:> > > > If the first remap plugin does not do a remap, then the URL is remapped > to> > > > the "from" URL in the remap rule, then the rest of the remap plugins > run.> > > > > > > > After:> > > > All remap plugins are run, and if *none* of them remap, then the "from" > URL> > > > from the remap rule is used.> > > > > > > > The primary point of this is to not have the first remap plugin be > special.> > > > In particular this means a plugin that's change from first to second> > > > doesn't see different behavior.> > > > > > > > > > > > On Mon, Dec 3, 2018 at 5:50 AM Takuya Kitano > <tk...@yahoo-corp.jp<mailto:tk...@yahoo-corp.jp>> wrote:> > > > > > > >> Thank you. This is exactly what I want to fix.> > > >> > > > >> > > > >> > > > >> 2018/12/02 21:59 に、"宋辰伟" <61...@qq.com<mailto:61...@qq.com>> を書き込みました:> > > >> > > > >> Actually there is a problem that URL is only overwritten in first> > > >> plugin in current logic. That means the request_url is overwritten in > first> > > >> plugin(which return NO_REMAP ) and never changed even if the remain > plugin> > > >> set map_to or map_form, except you set request_url directly. If my> > > >> understanding is correctly.> > > >> > > > >> scw00 - Song Chenwei> > > >> > > > >>> 在 2018年12月2日，上午11:26，Bret Palsson > <br...@gmail.com<mailto:br...@gmail.com>> 写道：> > > >>> > > > >>> I don’t normally comment, but feel like I need to chime in here.> > > >>> > > > >>> I agree in keeping with the stack behavior. This change has the> > > >> potential to break a lot of plugin logic and will cause a lot of work> > > >> across many dev teams that maintain many plugins. The pristine URL > should> > > >> be used in subsequent plugins as needed.> > > >>> > > > >>> Having said that, I am also interested to know and understand the> > > >> cause for wanting this behavior.> > > >>> > > > >>> -Bret> > > >>> > > > >>>> On Dec 1, 2018, at 20:15, Yongming Zhao > <mi...@gmail.com<mailto:mi...@gmail.com>> wrote:> > > >>>> > > > >>>> I’d like to take a little step back ask about what is the root> > > >> causing about this change:> > > >>>> > > > >>>> in the origin design> > > >>>> 1, the remap plugin works in ordered, which means you can> > > >> stop(consider it done) remapping at any plugin if you want, and the> > > >> following plugins will not run after.> > > >>>> 2, the remap plugin works in stacks, which means the seconds plugin> > > >> will continue work on the URL which is rewritten in the first plugin.> > > >>>> > > > >>>> when you talking about the origin URL, there is always a pristine> > > >> URL you can copy, if you really want, so the second plugin can see the> > > >> origin un-mapped URL. this pristine URL is design for logging, but I > think> > > >> you can take it for your purpose if you like> > > >>>> > > > >>>> > > > >>>> so, here rise my question:> > > >>>> can the pristine URL full file your second plugin needs? if so, can> > > >> we keep the origin design?> > > >>>> > > > >>>> > > > >>>> - Yongming Zhao 赵永明> > > >>>> > > > >>>>> 在 2018年11月5日，上午11:06，Takuya Kitano > <tk...@yahoo-corp.jp<mailto:tk...@yahoo-corp.jp>> 写道：> > > >>>>> > > > >>>>> Hi,> > > >>>>> > > > >>>>> I’d like to propose a change about the timing that ATS runs remap> > > >> plugins.> > > >>>>> > > > >>>>> Currently, we configure remap.config like the following snippet,> > > >>>>> and each plugins use `rri->requestUrl`,> > > >>>>> the behavior of a remap plugin as the first plugin is different> > > >> from that as the second plugin.> > > >>>>> > > > >>>>> ```> > > >>>>> map http://before-remap.com/ http://after-remap.com/> > > >> @plugin=<first plugin>.so @pparam=... @plugin=<second plugin>.so > @pparam=...> > > >>>>> ```> > > >>>>> > > > >>>>> In detail, the first plugin get pre-remapped url information from> > > >> `rri->requestUrl`,> > > >>>>> but the second plugin get post-remapped one.> > > >>>>> > > > >>>>> The cause of this behavior is ATS executes> > > >> `url_rewrite_remap_request` function after running the first remap > plugin.> > > >>>>> > > > >>>>> My proposal is that ATS should execute all remap plugins and then> > > >> rewrite url.> > > >>>>> > > > >>>>> More ditails are in> > > >>>>> - Issue : https://github.com/apache/trafficserver/issues/2877> > > >>>>> - Pull Request : https://github.com/apache/trafficserver/pull/4531> > > >>>>> > > > >>>>> > > > >>>>> What do you think about this?> > > >>>>> > > > >>>>> Thank you.> > > >>>>> > > > >>>> > > > >>> > > > >>> .> > > >> > > > >> > > > >> > > > >> > > > >> > > > >> > > > > > > > > -- > > > > *Beware the fisherman who's casting out his line in to a dried up > riverbed.*> > > > *Oh don't try to tell him 'cause he won't believe. Throw some bread to > the> > > > ducks instead.*> > > > *It's easier that way. *- Genesis : Duke : VI 25-28> > > > > > > > > > >
