> On Apr 1, 2019, at 2:57 PM, Walt Karas <wka...@verizonmedia.com.INVALID>
> wrote:
>
> https://github.com/apache/trafficserver/pull/5217
Well, other than I seriously question this use of template (how smart are
compilers, will it optimize away the to_same_char() calls completely, I don’t
know), there might be a real concern here:
This changes the cache key as well, right?
If so, not only should the PR mention that, we also have to make sure the
release notes for v9.0.0 are clear on this. Albeit unlikely, a particular app
could get 0% cache hit at an upgrade, if for some reason they used
non-normalized scheme or host names in their URLs (bad on them, but we don’t
like surprises).
Question 1; Why isn’t the scheme parsed down to a WKS? E.g. URL_WKSIDX_HTTPS; /
URL_WKSIDX_HTTP?
Question 2: Since this changes the url_print() method, doesn’t this change
behavior for a lot more than just TSHttpTxnEffectiveUrlStringGet() ?
url_print() is used by a lot of functions, including url_string_get(), which is
used in even more code. If so, the PR title / subject ought to be more
descriptive, and not focus on this just one API.
I think I’m ok with the general concept of normalizing these fields (better for
cache hit ratios). Not stoked with the template.
— leif
