From fcfcb319b5d39a5179a019e11eae4754f406b145 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 15 Mar 2023 13:11:49 -0600 Subject: [PATCH 1/3] Fix incorrect response when attempting to use a reserved CHQP --- lib/go-tc/deliveryservices.go | 8 ++++++++ .../deliveryservice/deliveryservices.go | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index 2cdedb536a..db614e27b9 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -49,6 +49,14 @@ const MaxRangeSliceBlockSize = 33554432 // values of a Delivery Service's 'Active' property (v3.0+). type DeliveryServiceActiveState string +// These names of URL query string parameters are not allowed to be in a +// Delivery Service's "ConsistentHashQueryParams" set, because they collide with +// query string parameters reserved for use by Traffic Router. +const ( + ReservedConsistentHashingQueryParameterFormat = "format" + ReservedConsistentHashingQueryParameterTRRED = "trred" +) + // A DeliveryServiceActiveState describes the availability of Delivery Service // content from the perspective of caching servers and Traffic Routers. const ( diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index acb8b67787..623769f059 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -1714,10 +1714,20 @@ func validateTypeFields(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { "consistentHashQueryParams": validation.Validate(ds, validation.By(func(dsi interface{}) error { ds := dsi.(*tc.DeliveryServiceV5) - if len(ds.ConsistentHashQueryParams) == 0 || tc.DSType(typeName).IsHTTP() { + if len(ds.ConsistentHashQueryParams) == 0 { return nil } - return fmt.Errorf("consistentHashQueryParams not allowed for '%s' deliveryservice type", typeName) + if !tc.DSType(typeName).IsHTTP() { + return fmt.Errorf("consistentHashQueryParams not allowed for '%s' deliveryservice type", typeName) + } + + for _, param := range ds.ConsistentHashQueryParams { + if param == tc.ReservedConsistentHashingQueryParameterFormat || param == tc.ReservedConsistentHashingQueryParameterTRRED { + return fmt.Errorf("'%s' cannot be used in consistent hashing, because it is reserved for use by Traffic Router", param) + } + } + + return nil })), "initialDispersion": validation.Validate(ds.InitialDispersion, validation.By(requiredIfMatchesTypeName([]string{httpTypeRegexp}, typeName)), From 82fcf5f979e97730ac15d5aca98e804bfaf59d1d Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 15 Mar 2023 13:12:40 -0600 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcac2c78fb..23ae58ed8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#6695](https://github.com/apache/trafficcontrol/issues/6695) *Traffic Control Cache Config (t3c)* Directory creation was erroneously reporting an error when actually succeeding. - [#7411](https://github.com/apache/trafficcontrol/pull/7411) *Traffic Control Cache Config (t3c)* Fixed issue with wrong parent ordering with MSO non-topology delivery services. - [#7425](https://github.com/apache/trafficcontrol/pull/7425) *Traffic Control Cache Config (t3c)* Fixed issue with layered profile iteration being done in the wrong order. +- [#6385](https://github.com/apache/trafficcontrol/issues/6385) *Traffic Ops* Reserved consistentHashQueryParameters cause internal server error ## [7.0.0] - 2022-07-19 ### Added From 03197b7920e325a513e6be20a1e1fa67cc3b7912 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 5 Apr 2023 14:33:13 -0600 Subject: [PATCH 3/3] Add a reserved query string parameter that I missed --- lib/go-tc/deliveryservices.go | 5 +++-- .../traffic_ops_golang/deliveryservice/deliveryservices.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index db614e27b9..103a1ea772 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -53,8 +53,9 @@ type DeliveryServiceActiveState string // Delivery Service's "ConsistentHashQueryParams" set, because they collide with // query string parameters reserved for use by Traffic Router. const ( - ReservedConsistentHashingQueryParameterFormat = "format" - ReservedConsistentHashingQueryParameterTRRED = "trred" + ReservedConsistentHashingQueryParameterFormat = "format" + ReservedConsistentHashingQueryParameterTRRED = "trred" + ReservedConsistentHashingQueryParameterFakeClientIPAddress = "fakeClientIpAddress" ) // A DeliveryServiceActiveState describes the availability of Delivery Service diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index 623769f059..c1c6943351 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -1722,7 +1722,7 @@ func validateTypeFields(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { } for _, param := range ds.ConsistentHashQueryParams { - if param == tc.ReservedConsistentHashingQueryParameterFormat || param == tc.ReservedConsistentHashingQueryParameterTRRED { + if param == tc.ReservedConsistentHashingQueryParameterFormat || param == tc.ReservedConsistentHashingQueryParameterTRRED || param == tc.ReservedConsistentHashingQueryParameterFakeClientIPAddress { return fmt.Errorf("'%s' cannot be used in consistent hashing, because it is reserved for use by Traffic Router", param) } }