This is an automated email from the ASF dual-hosted git repository.

zrhoffman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new 94d9850fe4 Fix reserved consistentHashQueryParams entries causing 
internal server error instead of client error (#7405)
94d9850fe4 is described below

commit 94d9850fe48a7714e33d1e6f71c8e1479ec7c694
Author: ocket8888 <[email protected]>
AuthorDate: Wed Apr 5 15:18:07 2023 -0600

    Fix reserved consistentHashQueryParams entries causing internal server 
error instead of client error (#7405)
    
    * Fix incorrect response when attempting to use a reserved CHQP
    
    * Update changelog
    
    * Add a reserved query string parameter that I missed
---
 CHANGELOG.md                                               |  1 +
 lib/go-tc/deliveryservices.go                              |  9 +++++++++
 .../traffic_ops_golang/deliveryservice/deliveryservices.go | 14 ++++++++++++--
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 02653b498c..ec37e0a781 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
 
 ### Removed
 - [#7271](https://github.com/apache/trafficcontrol/pull/7271) Remove 
components in `infrastructre/docker/`, not in use as cdn-in-a-box performs the 
same functionality.
diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go
index 2cdedb536a..103a1ea772 100644
--- a/lib/go-tc/deliveryservices.go
+++ b/lib/go-tc/deliveryservices.go
@@ -49,6 +49,15 @@ 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"
+       ReservedConsistentHashingQueryParameterFakeClientIPAddress = 
"fakeClientIpAddress"
+)
+
 // 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..c1c6943351 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 || param == 
tc.ReservedConsistentHashingQueryParameterFakeClientIPAddress {
+                                               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)),

Reply via email to