You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ra...@apache.org on 2020/11/02 20:50:28 UTC
[trafficcontrol] branch 4.1.x updated: Fix ORT range directive
duplication (#4877) (#5234)
This is an automated email from the ASF dual-hosted git repository.
rawlin pushed a commit to branch 4.1.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/4.1.x by this push:
new 841df36 Fix ORT range directive duplication (#4877) (#5234)
841df36 is described below
commit 841df363fc675ede82ce64dac46d8a79022e40f4
Author: Rawlin Peters <ra...@apache.org>
AuthorDate: Mon Nov 2 13:50:10 2020 -0700
Fix ORT range directive duplication (#4877) (#5234)
Fixes a bug where DSes with multiple remap lines (e.g. HTTP and HTTPS)
result in a double range directive, if there's a __RANGE_DIRECTIVE__
override.
(cherry picked from commit 533b8e4d83eac78b9c398f659ba5ec2b475d1a52)
Co-authored-by: Robert O Butts <ro...@users.noreply.github.com>
---
lib/go-atscfg/remapdotconfig.go | 15 ++++++++++-----
lib/go-atscfg/remapdotconfig_test.go | 13 ++++++++-----
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/lib/go-atscfg/remapdotconfig.go b/lib/go-atscfg/remapdotconfig.go
index 2eaf2df..bdcb6f7 100644
--- a/lib/go-atscfg/remapdotconfig.go
+++ b/lib/go-atscfg/remapdotconfig.go
@@ -221,7 +221,6 @@ const RemapConfigRangeDirective = `__RANGE_DIRECTIVE__`
// The cacheKeyConfigParams map may be nil, if this ds profile had no cache key config params.
func BuildRemapLine(cacheURLConfigParams map[string]string, atsMajorVersion int, server *ServerInfo, pData map[string]string, text string, ds RemapConfigDSData, mapFrom string, mapTo string, cacheKeyConfigParams map[string]string) string {
// ds = 'remap' in perl
-
mapFrom = strings.Replace(mapFrom, `__http__`, server.HostName, -1)
if _, hasDSCPRemap := pData["dscp_remap"]; hasDSCPRemap {
@@ -306,14 +305,20 @@ func BuildRemapLine(cacheURLConfigParams map[string]string, atsMajorVersion int,
rangeReqTxt = ` @plugin=slice.so @pparam=--blockbytes=` + strconv.Itoa(*ds.RangeSliceBlockSize) + ` @plugin=cache_range_requests.so `
}
}
- if ds.RemapText != nil && *ds.RemapText != "" && strings.Contains(*ds.RemapText, RemapConfigRangeDirective) {
- *ds.RemapText = strings.Replace(*ds.RemapText, `__RANGE_DIRECTIVE__`, rangeReqTxt, 1)
+
+ remapText := ""
+ if ds.RemapText != nil {
+ remapText = *ds.RemapText
+ }
+
+ if strings.Contains(remapText, RemapConfigRangeDirective) {
+ remapText = strings.Replace(remapText, RemapConfigRangeDirective, rangeReqTxt, 1)
} else {
text += rangeReqTxt
}
- if ds.RemapText != nil && *ds.RemapText != "" {
- text += " " + *ds.RemapText
+ if remapText != "" {
+ text += " " + remapText
}
if ds.FQPacingRate != nil && *ds.FQPacingRate > 0 {
diff --git a/lib/go-atscfg/remapdotconfig_test.go b/lib/go-atscfg/remapdotconfig_test.go
index 2030b43..36b922f 100644
--- a/lib/go-atscfg/remapdotconfig_test.go
+++ b/lib/go-atscfg/remapdotconfig_test.go
@@ -4940,7 +4940,7 @@ func TestMakeRemapDotConfigRawRemapRangeDirective(t *testing.T) {
RegexSetNumber: util.StrPtr("myregexsetnum"),
OriginShield: util.StrPtr("myoriginshield"),
ProfileID: util.IntPtr(49),
- Protocol: util.IntPtr(int(tc.DSProtocolHTTPToHTTPS)),
+ Protocol: util.IntPtr(int(tc.DSProtocolHTTPAndHTTPS)),
AnonymousBlockingEnabled: util.BoolPtr(false),
Active: true,
RangeSliceBlockSize: util.IntPtr(262144),
@@ -4955,8 +4955,8 @@ func TestMakeRemapDotConfigRawRemapRangeDirective(t *testing.T) {
txtLines := strings.Split(txt, "\n")
- if len(txtLines) != 2 {
- t.Fatalf("expected 1 remaps from HTTP_TO_HTTPS DS, actual: '%v' count %v", txt, len(txtLines))
+ if len(txtLines) != 3 { // 2 remaps plus header comment
+ t.Fatalf("expected 2 remaps from HTTP_AND_HTTPS DS, actual: '%v' count %v", txt, len(txtLines))
}
remapLine := txtLines[1]
@@ -4983,8 +4983,11 @@ func TestMakeRemapDotConfigRawRemapRangeDirective(t *testing.T) {
if strings.Contains(remapLine, "__RANGE_DIRECTIVE__") {
t.Errorf("expected raw remap range directive to be replaced, actual '%v'", txt)
}
- if strings.Count(remapLine, "slice.so") != 1 {
- t.Errorf("expected raw remap range directive to be replaced not duplicated, actual '%v'", txt)
+ if count := strings.Count(remapLine, "slice.so"); count != 1 { // Individual line should only have 1 slice.so
+ t.Errorf("expected raw remap range directive to be replaced not duplicated, actual count %v '%v'", count, txt)
+ }
+ if count := strings.Count(txt, "slice.so"); count != 2 { // All lines should have 2 slice.so - HTTP and HTTPS lines
+ t.Errorf("expected raw remap range directive to have one slice.so for HTTP and one for HTTPS remap, actual count %v '%v'", count, txt)
}
}