alficles commented on a change in pull request #2590: Add s? to cachkey capture
prefix so it will capture both http and https in to http keys
URL: https://github.com/apache/trafficcontrol/pull/2590#discussion_r205438698
##########
File path: traffic_ops/app/lib/UI/DeliveryService.pm
##########
@@ -1432,7 +1432,7 @@ sub get_qstring_ignore_remap {
my $ats_major_version = shift;
if ($ats_major_version >= 6) {
- return " \@plugin=cachekey.so \@pparam=--separator=
\@pparam=--remove-all-params=true \@pparam=--remove-path=true
\@pparam=--capture-prefix-uri=/http:\\/\\/([^?]*)/http:\\/\\/\$1/";
+ return " \@plugin=cachekey.so \@pparam=--separator=
\@pparam=--remove-all-params=true \@pparam=--remove-path=true
\@pparam=--capture-prefix-uri=/https?:\\/\\/([^?]*)/http:\\/\\/\$1/";
Review comment:
I think this isn't quite right. An origin may legitimately serve different
content on its http and https ports, but this conflates the two into a single
cachekey. The scheme should be part of the key. Might `/^([^?]*)/$1/` work
better, to simply include the scheme, which we know won't have a `?` in it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
