diff --git a/.changelog/13256.txt b/.changelog/13256.txt new file mode 100644 index 000000000..cfc8096e3 --- /dev/null +++ b/.changelog/13256.txt @@ -0,0 +1,3 @@ +```release-note:bug +agent: Fixed a bug in HTTP handlers where URLs were being decoded twice +``` \ No newline at end of file diff --git a/agent/acl_endpoint.go b/agent/acl_endpoint.go index ef859f84e..23e43ba5c 100644 --- a/agent/acl_endpoint.go +++ b/agent/acl_endpoint.go @@ -122,10 +122,7 @@ func (s *HTTPHandlers) ACLPolicyCRUD(resp http.ResponseWriter, req *http.Request return nil, MethodNotAllowedError{req.Method, []string{"GET", "PUT", "DELETE"}} } - policyID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/acl/policy/") - if err != nil { - return nil, err - } + policyID := strings.TrimPrefix(req.URL.Path, "/v1/acl/policy/") if policyID == "" && req.Method != "PUT" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing policy ID"} } @@ -170,10 +167,7 @@ func (s *HTTPHandlers) ACLPolicyReadByName(resp http.ResponseWriter, req *http.R return nil, aclDisabled } - policyName, err := getPathSuffixUnescaped(req.URL.Path, "/v1/acl/policy/name/") - if err != nil { - return nil, err - } + policyName := strings.TrimPrefix(req.URL.Path, "/v1/acl/policy/name/") if policyName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing policy Name"} } @@ -308,10 +302,7 @@ func (s *HTTPHandlers) ACLTokenCRUD(resp http.ResponseWriter, req *http.Request) return nil, MethodNotAllowedError{req.Method, []string{"GET", "PUT", "DELETE"}} } - tokenID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/acl/token/") - if err != nil { - return nil, err - } + tokenID := strings.TrimPrefix(req.URL.Path, "/v1/acl/token/") if strings.HasSuffix(tokenID, "/clone") && req.Method == "PUT" { tokenID = tokenID[:len(tokenID)-6] fn = s.ACLTokenClone @@ -541,10 +532,7 @@ func (s *HTTPHandlers) ACLRoleCRUD(resp http.ResponseWriter, req *http.Request) return nil, MethodNotAllowedError{req.Method, []string{"GET", "PUT", "DELETE"}} } - roleID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/acl/role/") - if err != nil { - return nil, err - } + roleID := strings.TrimPrefix(req.URL.Path, "/v1/acl/role/") if roleID == "" && req.Method != "PUT" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing role ID"} } @@ -557,10 +545,7 @@ func (s *HTTPHandlers) ACLRoleReadByName(resp http.ResponseWriter, req *http.Req return nil, aclDisabled } - roleName, err := getPathSuffixUnescaped(req.URL.Path, "/v1/acl/role/name/") - if err != nil { - return nil, err - } + roleName := strings.TrimPrefix(req.URL.Path, "/v1/acl/role/name/") if roleName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing role Name"} } @@ -711,10 +696,7 @@ func (s *HTTPHandlers) ACLBindingRuleCRUD(resp http.ResponseWriter, req *http.Re return nil, MethodNotAllowedError{req.Method, []string{"GET", "PUT", "DELETE"}} } - bindingRuleID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/acl/binding-rule/") - if err != nil { - return nil, err - } + bindingRuleID := strings.TrimPrefix(req.URL.Path, "/v1/acl/binding-rule/") if bindingRuleID == "" && req.Method != "PUT" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing binding rule ID"} } @@ -857,10 +839,7 @@ func (s *HTTPHandlers) ACLAuthMethodCRUD(resp http.ResponseWriter, req *http.Req return nil, MethodNotAllowedError{req.Method, []string{"GET", "PUT", "DELETE"}} } - methodName, err := getPathSuffixUnescaped(req.URL.Path, "/v1/acl/auth-method/") - if err != nil { - return nil, err - } + methodName := strings.TrimPrefix(req.URL.Path, "/v1/acl/auth-method/") if methodName == "" && req.Method != "PUT" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing auth method name"} } diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 87805a24a..86197f462 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -382,10 +382,7 @@ func (s *HTTPHandlers) AgentServices(resp http.ResponseWriter, req *http.Request // blocking watch using hash-based blocking. func (s *HTTPHandlers) AgentService(resp http.ResponseWriter, req *http.Request) (interface{}, error) { // Get the proxy ID. Note that this is the ID of a proxy's service instance. - id, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/service/") - if err != nil { - return nil, err - } + id := strings.TrimPrefix(req.URL.Path, "/v1/agent/service/") // Maybe block var queryOpts structs.QueryOptions @@ -404,7 +401,7 @@ func (s *HTTPHandlers) AgentService(resp http.ResponseWriter, req *http.Request) } // need to resolve to default the meta - _, err = s.agent.delegate.ResolveTokenAndDefaultMeta(token, &entMeta, nil) + _, err := s.agent.delegate.ResolveTokenAndDefaultMeta(token, &entMeta, nil) if err != nil { return nil, err } @@ -640,10 +637,7 @@ func (s *HTTPHandlers) AgentJoin(resp http.ResponseWriter, req *http.Request) (i } // Get the address - addr, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/join/") - if err != nil { - return nil, err - } + addr := strings.TrimPrefix(req.URL.Path, "/v1/agent/join/") if wan { if s.agent.config.ConnectMeshGatewayWANFederationEnabled { @@ -703,10 +697,7 @@ func (s *HTTPHandlers) AgentForceLeave(resp http.ResponseWriter, req *http.Reque // Check if the WAN is being queried _, wan := req.URL.Query()["wan"] - addr, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/force-leave/") - if err != nil { - return nil, err - } + addr := strings.TrimPrefix(req.URL.Path, "/v1/agent/force-leave/") if wan { return nil, s.agent.ForceLeaveWAN(addr, prune, entMeta) } else { @@ -792,11 +783,8 @@ func (s *HTTPHandlers) AgentRegisterCheck(resp http.ResponseWriter, req *http.Re } func (s *HTTPHandlers) AgentDeregisterCheck(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - ID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/check/deregister/") - if err != nil { - return nil, err - } - checkID := structs.NewCheckID(types.CheckID(ID), nil) + id := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/deregister/") + checkID := structs.NewCheckID(types.CheckID(id), nil) // Get the provided token, if any, and vet against any ACL policies. var token string @@ -829,21 +817,15 @@ func (s *HTTPHandlers) AgentDeregisterCheck(resp http.ResponseWriter, req *http. } func (s *HTTPHandlers) AgentCheckPass(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - ID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/check/pass/") - if err != nil { - return nil, err - } - checkID := types.CheckID(ID) + id := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/pass/") + checkID := types.CheckID(id) note := req.URL.Query().Get("note") return s.agentCheckUpdate(resp, req, checkID, api.HealthPassing, note) } func (s *HTTPHandlers) AgentCheckWarn(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - ID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/check/warn/") - if err != nil { - return nil, err - } - checkID := types.CheckID(ID) + id := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/warn/") + checkID := types.CheckID(id) note := req.URL.Query().Get("note") return s.agentCheckUpdate(resp, req, checkID, api.HealthWarning, note) @@ -851,11 +833,8 @@ func (s *HTTPHandlers) AgentCheckWarn(resp http.ResponseWriter, req *http.Reques } func (s *HTTPHandlers) AgentCheckFail(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - ID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/check/fail/") - if err != nil { - return nil, err - } - checkID := types.CheckID(ID) + id := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/fail/") + checkID := types.CheckID(id) note := req.URL.Query().Get("note") return s.agentCheckUpdate(resp, req, checkID, api.HealthCritical, note) @@ -890,12 +869,8 @@ func (s *HTTPHandlers) AgentCheckUpdate(resp http.ResponseWriter, req *http.Requ return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: fmt.Sprintf("Invalid check status: '%s'", update.Status)} } - ID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/check/update/") - if err != nil { - return nil, err - } - - checkID := types.CheckID(ID) + id := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/update/") + checkID := types.CheckID(id) return s.agentCheckUpdate(resp, req, checkID, update.Status, update.Output) } @@ -977,10 +952,7 @@ func returnTextPlain(req *http.Request) bool { // AgentHealthServiceByID return the local Service Health given its ID func (s *HTTPHandlers) AgentHealthServiceByID(resp http.ResponseWriter, req *http.Request) (interface{}, error) { // Pull out the service id (service id since there may be several instance of the same service on this host) - serviceID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/health/service/id/") - if err != nil { - return nil, err - } + serviceID := strings.TrimPrefix(req.URL.Path, "/v1/agent/health/service/id/") if serviceID == "" { return nil, &HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing serviceID"} } @@ -1038,11 +1010,7 @@ func (s *HTTPHandlers) AgentHealthServiceByID(resp http.ResponseWriter, req *htt // AgentHealthServiceByName return the worse status of all the services with given name on an agent func (s *HTTPHandlers) AgentHealthServiceByName(resp http.ResponseWriter, req *http.Request) (interface{}, error) { // Pull out the service name - serviceName, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/health/service/name/") - if err != nil { - return nil, err - } - + serviceName := strings.TrimPrefix(req.URL.Path, "/v1/agent/health/service/name/") if serviceName == "" { return nil, &HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing service Name"} } @@ -1247,11 +1215,7 @@ func (s *HTTPHandlers) AgentRegisterService(resp http.ResponseWriter, req *http. } func (s *HTTPHandlers) AgentDeregisterService(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - serviceID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/service/deregister/") - if err != nil { - return nil, err - } - + serviceID := strings.TrimPrefix(req.URL.Path, "/v1/agent/service/deregister/") sid := structs.NewServiceID(serviceID, nil) // Get the provided token, if any, and vet against any ACL policies. @@ -1287,11 +1251,7 @@ func (s *HTTPHandlers) AgentDeregisterService(resp http.ResponseWriter, req *htt func (s *HTTPHandlers) AgentServiceMaintenance(resp http.ResponseWriter, req *http.Request) (interface{}, error) { // Ensure we have a service ID - serviceID, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/service/maintenance/") - if err != nil { - return nil, err - } - + serviceID := strings.TrimPrefix(req.URL.Path, "/v1/agent/service/maintenance/") sid := structs.NewServiceID(serviceID, nil) if sid.ID == "" { @@ -1489,10 +1449,7 @@ func (s *HTTPHandlers) AgentToken(resp http.ResponseWriter, req *http.Request) ( } // Figure out the target token. - target, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/token/") - if err != nil { - return nil, err - } + target := strings.TrimPrefix(req.URL.Path, "/v1/agent/token/") err = s.agent.tokens.WithPersistenceLock(func() error { triggerAntiEntropySync := false @@ -1565,10 +1522,7 @@ func (s *HTTPHandlers) AgentConnectCARoots(resp http.ResponseWriter, req *http.R func (s *HTTPHandlers) AgentConnectCALeafCert(resp http.ResponseWriter, req *http.Request) (interface{}, error) { // Get the service name. Note that this is the name of the service, // not the ID of the service instance. - serviceName, err := getPathSuffixUnescaped(req.URL.Path, "/v1/agent/connect/ca/leaf/") - if err != nil { - return nil, err - } + serviceName := strings.TrimPrefix(req.URL.Path, "/v1/agent/connect/ca/leaf/") args := cachetype.ConnectCALeafRequest{ Service: serviceName, // Need name not ID diff --git a/agent/catalog_endpoint.go b/agent/catalog_endpoint.go index 3617a3d3a..fbee9cbfc 100644 --- a/agent/catalog_endpoint.go +++ b/agent/catalog_endpoint.go @@ -3,6 +3,7 @@ package agent import ( "fmt" "net/http" + "strings" metrics "github.com/armon/go-metrics" "github.com/armon/go-metrics/prometheus" @@ -361,11 +362,7 @@ func (s *HTTPHandlers) catalogServiceNodes(resp http.ResponseWriter, req *http.R } // Pull out the service name - var err error - args.ServiceName, err = getPathSuffixUnescaped(req.URL.Path, pathPrefix) - if err != nil { - return nil, err - } + args.ServiceName = strings.TrimPrefix(req.URL.Path, pathPrefix) if args.ServiceName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing service name"} } @@ -436,11 +433,7 @@ func (s *HTTPHandlers) CatalogNodeServices(resp http.ResponseWriter, req *http.R } // Pull out the node name - var err error - args.Node, err = getPathSuffixUnescaped(req.URL.Path, "/v1/catalog/node/") - if err != nil { - return nil, err - } + args.Node = strings.TrimPrefix(req.URL.Path, "/v1/catalog/node/") if args.Node == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing node name"} } @@ -501,11 +494,7 @@ func (s *HTTPHandlers) CatalogNodeServiceList(resp http.ResponseWriter, req *htt } // Pull out the node name - var err error - args.Node, err = getPathSuffixUnescaped(req.URL.Path, "/v1/catalog/node-services/") - if err != nil { - return nil, err - } + args.Node = strings.TrimPrefix(req.URL.Path, "/v1/catalog/node-services/") if args.Node == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing node name"} } @@ -552,11 +541,7 @@ func (s *HTTPHandlers) CatalogGatewayServices(resp http.ResponseWriter, req *htt } // Pull out the gateway's service name - var err error - args.ServiceName, err = getPathSuffixUnescaped(req.URL.Path, "/v1/catalog/gateway-services/") - if err != nil { - return nil, err - } + args.ServiceName = strings.TrimPrefix(req.URL.Path, "/v1/catalog/gateway-services/") if args.ServiceName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing gateway name"} } diff --git a/agent/config_endpoint.go b/agent/config_endpoint.go index b51c1e273..7e67d851a 100644 --- a/agent/config_endpoint.go +++ b/agent/config_endpoint.go @@ -33,10 +33,7 @@ func (s *HTTPHandlers) configGet(resp http.ResponseWriter, req *http.Request) (i if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done { return nil, nil } - kindAndName, err := getPathSuffixUnescaped(req.URL.Path, "/v1/config/") - if err != nil { - return nil, err - } + kindAndName := strings.TrimPrefix(req.URL.Path, "/v1/config/") pathArgs := strings.SplitN(kindAndName, "/", 2) switch len(pathArgs) { @@ -84,10 +81,7 @@ func (s *HTTPHandlers) configDelete(resp http.ResponseWriter, req *http.Request) var args structs.ConfigEntryRequest s.parseDC(req, &args.Datacenter) s.parseToken(req, &args.Token) - kindAndName, err := getPathSuffixUnescaped(req.URL.Path, "/v1/config/") - if err != nil { - return nil, err - } + kindAndName := strings.TrimPrefix(req.URL.Path, "/v1/config/") pathArgs := strings.SplitN(kindAndName, "/", 2) if len(pathArgs) != 2 { diff --git a/agent/coordinate_endpoint.go b/agent/coordinate_endpoint.go index 14176ab1e..bb4f328ed 100644 --- a/agent/coordinate_endpoint.go +++ b/agent/coordinate_endpoint.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "sort" + "strings" "github.com/hashicorp/consul/agent/structs" ) @@ -99,10 +100,7 @@ func (s *HTTPHandlers) CoordinateNode(resp http.ResponseWriter, req *http.Reques return nil, err } - node, err := getPathSuffixUnescaped(req.URL.Path, "/v1/coordinate/node/") - if err != nil { - return nil, err - } + node := strings.TrimPrefix(req.URL.Path, "/v1/coordinate/node/") args := structs.NodeSpecificRequest{Node: node} if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done { return nil, nil diff --git a/agent/discovery_chain_endpoint.go b/agent/discovery_chain_endpoint.go index 475ef02d6..4f762faf0 100644 --- a/agent/discovery_chain_endpoint.go +++ b/agent/discovery_chain_endpoint.go @@ -3,6 +3,7 @@ package agent import ( "fmt" "net/http" + "strings" "time" "github.com/mitchellh/mapstructure" @@ -19,11 +20,7 @@ func (s *HTTPHandlers) DiscoveryChainRead(resp http.ResponseWriter, req *http.Re return nil, nil } - var err error - args.Name, err = getPathSuffixUnescaped(req.URL.Path, "/v1/discovery-chain/") - if err != nil { - return nil, err - } + args.Name = strings.TrimPrefix(req.URL.Path, "/v1/discovery-chain/") if args.Name == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing chain name"} } diff --git a/agent/event_endpoint.go b/agent/event_endpoint.go index 78be383d8..971cb9deb 100644 --- a/agent/event_endpoint.go +++ b/agent/event_endpoint.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "strconv" + "strings" "time" "github.com/hashicorp/consul/acl" @@ -19,11 +20,7 @@ func (s *HTTPHandlers) EventFire(resp http.ResponseWriter, req *http.Request) (i s.parseDC(req, &dc) event := &UserEvent{} - var err error - event.Name, err = getPathSuffixUnescaped(req.URL.Path, "/v1/event/fire/") - if err != nil { - return nil, err - } + event.Name = strings.TrimPrefix(req.URL.Path, "/v1/event/fire/") if event.Name == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing name"} } diff --git a/agent/federation_state_endpoint.go b/agent/federation_state_endpoint.go index aecb58ff3..4b7757ef8 100644 --- a/agent/federation_state_endpoint.go +++ b/agent/federation_state_endpoint.go @@ -2,16 +2,14 @@ package agent import ( "net/http" + "strings" "github.com/hashicorp/consul/agent/structs" ) // GET /v1/internal/federation-state/ func (s *HTTPHandlers) FederationStateGet(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - datacenterName, err := getPathSuffixUnescaped(req.URL.Path, "/v1/internal/federation-state/") - if err != nil { - return nil, err - } + datacenterName := strings.TrimPrefix(req.URL.Path, "/v1/internal/federation-state/") if datacenterName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing datacenter name"} } diff --git a/agent/health_endpoint.go b/agent/health_endpoint.go index c8b8cf15c..6ea64528b 100644 --- a/agent/health_endpoint.go +++ b/agent/health_endpoint.go @@ -4,6 +4,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" @@ -28,11 +29,7 @@ func (s *HTTPHandlers) HealthChecksInState(resp http.ResponseWriter, req *http.R } // Pull out the service name - var err error - args.State, err = getPathSuffixUnescaped(req.URL.Path, "/v1/health/state/") - if err != nil { - return nil, err - } + args.State = strings.TrimPrefix(req.URL.Path, "/v1/health/state/") if args.State == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing check state"} } @@ -76,11 +73,7 @@ func (s *HTTPHandlers) HealthNodeChecks(resp http.ResponseWriter, req *http.Requ } // Pull out the service name - var err error - args.Node, err = getPathSuffixUnescaped(req.URL.Path, "/v1/health/node/") - if err != nil { - return nil, err - } + args.Node = strings.TrimPrefix(req.URL.Path, "/v1/health/node/") if args.Node == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing node name"} } @@ -126,11 +119,7 @@ func (s *HTTPHandlers) HealthServiceChecks(resp http.ResponseWriter, req *http.R } // Pull out the service name - var err error - args.ServiceName, err = getPathSuffixUnescaped(req.URL.Path, "/v1/health/checks/") - if err != nil { - return nil, err - } + args.ServiceName = strings.TrimPrefix(req.URL.Path, "/v1/health/checks/") if args.ServiceName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing service name"} } @@ -222,11 +211,7 @@ func (s *HTTPHandlers) healthServiceNodes(resp http.ResponseWriter, req *http.Re } // Pull out the service name - var err error - args.ServiceName, err = getPathSuffixUnescaped(req.URL.Path, prefix) - if err != nil { - return nil, err - } + args.ServiceName = strings.TrimPrefix(req.URL.Path, prefix) if args.ServiceName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing service name"} } diff --git a/agent/http.go b/agent/http.go index 9bb5ed340..5dbcdf00b 100644 --- a/agent/http.go +++ b/agent/http.go @@ -1139,18 +1139,6 @@ func (s *HTTPHandlers) parseFilter(req *http.Request, filter *string) { } } -func getPathSuffixUnescaped(path string, prefixToTrim string) (string, error) { - // The suffix may be URL-encoded, so attempt to decode - suffixRaw := strings.TrimPrefix(path, prefixToTrim) - suffixUnescaped, err := url.PathUnescape(suffixRaw) - - if err != nil { - return suffixRaw, fmt.Errorf("failure in unescaping path param %q: %v", suffixRaw, err) - } - - return suffixUnescaped, nil -} - func setMetaProtobuf(resp http.ResponseWriter, queryMeta *pbcommon.QueryMeta) { qm := new(structs.QueryMeta) pbcommon.QueryMetaToStructs(queryMeta, qm) diff --git a/agent/http_test.go b/agent/http_test.go index 525c4675a..a42a93230 100644 --- a/agent/http_test.go +++ b/agent/http_test.go @@ -1686,42 +1686,3 @@ func TestRPC_HTTPSMaxConnsPerClient(t *testing.T) { }) } } - -func TestGetPathSuffixUnescaped(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - pathInput string - pathPrefix string - suffixResult string - errString string - }{ - // No decoding required (resource name must be unaffected by the decode) - {"Normal Valid", "/foo/bar/resource-1", "/foo/bar/", "resource-1", ""}, - // This function is not responsible for enforcing a valid URL, just for decoding escaped values. - // If there's an invalid URL segment in the path, it will be returned as is. - {"Unencoded Invalid", "/foo/bar/resource 1", "/foo/bar/", "resource 1", ""}, - // Decode the encoded value properly - {"Encoded Valid", "/foo/bar/re%2Fsource%201", "/foo/bar/", "re/source 1", ""}, - // Fail to decode an invalidly encoded input - {"Encoded Invalid", "/foo/bar/re%Fsource%201", "/foo/bar/", "re%Fsource%201", "failure in unescaping path param"}, - } - - for _, tc := range cases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - - suffixResult, err := getPathSuffixUnescaped(tc.pathInput, tc.pathPrefix) - - require.Equal(t, suffixResult, tc.suffixResult) - - if tc.errString == "" { - require.NoError(t, err) - } else { - require.Error(t, err) - require.Contains(t, err.Error(), tc.errString) - } - }) - } -} diff --git a/agent/intentions_endpoint.go b/agent/intentions_endpoint.go index ff720e900..d21833a8f 100644 --- a/agent/intentions_endpoint.go +++ b/agent/intentions_endpoint.go @@ -486,10 +486,7 @@ func parseIntentionStringComponent(input string, entMeta *acl.EnterpriseMeta) (s // IntentionSpecific handles the endpoint for /v1/connect/intentions/:id. // Deprecated: use IntentionExact. func (s *HTTPHandlers) IntentionSpecific(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - id, err := getPathSuffixUnescaped(req.URL.Path, "/v1/connect/intentions/") - if err != nil { - return nil, err - } + id := strings.TrimPrefix(req.URL.Path, "/v1/connect/intentions/") switch req.Method { case "GET": diff --git a/agent/kvs_endpoint.go b/agent/kvs_endpoint.go index 4ac9f4119..bcb874588 100644 --- a/agent/kvs_endpoint.go +++ b/agent/kvs_endpoint.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "strconv" + "strings" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" @@ -19,11 +20,7 @@ func (s *HTTPHandlers) KVSEndpoint(resp http.ResponseWriter, req *http.Request) } // Pull out the key name, validation left to each sub-handler - var err error - args.Key, err = getPathSuffixUnescaped(req.URL.Path, "/v1/kv/") - if err != nil { - return nil, err - } + args.Key = strings.TrimPrefix(req.URL.Path, "/v1/kv/") // Check for a key list keyList := false diff --git a/agent/peering_endpoint.go b/agent/peering_endpoint.go index 569c8ca4a..162d8985e 100644 --- a/agent/peering_endpoint.go +++ b/agent/peering_endpoint.go @@ -3,6 +3,7 @@ package agent import ( "fmt" "net/http" + "strings" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/api" @@ -12,10 +13,7 @@ import ( // PeeringEndpoint handles GET, DELETE on v1/peering/name func (s *HTTPHandlers) PeeringEndpoint(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - name, err := getPathSuffixUnescaped(req.URL.Path, "/v1/peering/") - if err != nil { - return nil, err - } + name := strings.TrimPrefix(req.URL.Path, "/v1/peering/") if name == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Must specify a name to fetch."} } diff --git a/agent/prepared_query_endpoint.go b/agent/prepared_query_endpoint.go index f6cbe6494..6f5e7a9c2 100644 --- a/agent/prepared_query_endpoint.go +++ b/agent/prepared_query_endpoint.go @@ -309,10 +309,7 @@ func (s *HTTPHandlers) PreparedQuerySpecific(resp http.ResponseWriter, req *http } path := req.URL.Path - id, err := getPathSuffixUnescaped(path, "/v1/query/") - if err != nil { - return nil, err - } + id := strings.TrimPrefix(path, "/v1/query/") switch { case strings.HasSuffix(path, "/execute"): diff --git a/agent/session_endpoint.go b/agent/session_endpoint.go index 74228cbb1..d1b6dd7cf 100644 --- a/agent/session_endpoint.go +++ b/agent/session_endpoint.go @@ -3,6 +3,7 @@ package agent import ( "fmt" "net/http" + "strings" "time" "github.com/hashicorp/consul/agent/structs" @@ -69,11 +70,7 @@ func (s *HTTPHandlers) SessionDestroy(resp http.ResponseWriter, req *http.Reques } // Pull out the session id - var err error - args.Session.ID, err = getPathSuffixUnescaped(req.URL.Path, "/v1/session/destroy/") - if err != nil { - return nil, err - } + args.Session.ID = strings.TrimPrefix(req.URL.Path, "/v1/session/destroy/") if args.Session.ID == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing session"} } @@ -96,11 +93,7 @@ func (s *HTTPHandlers) SessionRenew(resp http.ResponseWriter, req *http.Request) } // Pull out the session id - var err error - args.SessionID, err = getPathSuffixUnescaped(req.URL.Path, "/v1/session/renew/") - if err != nil { - return nil, err - } + args.SessionID = strings.TrimPrefix(req.URL.Path, "/v1/session/renew/") args.Session = args.SessionID if args.SessionID == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing session"} @@ -127,11 +120,7 @@ func (s *HTTPHandlers) SessionGet(resp http.ResponseWriter, req *http.Request) ( } // Pull out the session id - var err error - args.SessionID, err = getPathSuffixUnescaped(req.URL.Path, "/v1/session/info/") - if err != nil { - return nil, err - } + args.SessionID = strings.TrimPrefix(req.URL.Path, "/v1/session/info/") args.Session = args.SessionID if args.SessionID == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing session"} @@ -184,11 +173,7 @@ func (s *HTTPHandlers) SessionsForNode(resp http.ResponseWriter, req *http.Reque } // Pull out the node name - var err error - args.Node, err = getPathSuffixUnescaped(req.URL.Path, "/v1/session/node/") - if err != nil { - return nil, err - } + args.Node = strings.TrimPrefix(req.URL.Path, "/v1/session/node/") if args.Node == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing node name"} } diff --git a/agent/ui_endpoint.go b/agent/ui_endpoint.go index 1a8d5591a..70d5d9def 100644 --- a/agent/ui_endpoint.go +++ b/agent/ui_endpoint.go @@ -134,11 +134,7 @@ func (s *HTTPHandlers) UINodeInfo(resp http.ResponseWriter, req *http.Request) ( } // Verify we have some DC, or use the default - var err error - args.Node, err = getPathSuffixUnescaped(req.URL.Path, "/v1/internal/ui/node/") - if err != nil { - return nil, err - } + args.Node = strings.TrimPrefix(req.URL.Path, "/v1/internal/ui/node/") if args.Node == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing node name"} } @@ -266,11 +262,7 @@ func (s *HTTPHandlers) UIGatewayServicesNodes(resp http.ResponseWriter, req *htt } // Pull out the service name - var err error - args.ServiceName, err = getPathSuffixUnescaped(req.URL.Path, "/v1/internal/ui/gateway-services-nodes/") - if err != nil { - return nil, err - } + args.ServiceName = strings.TrimPrefix(req.URL.Path, "/v1/internal/ui/gateway-services-nodes/") if args.ServiceName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing gateway name"} } @@ -310,11 +302,7 @@ func (s *HTTPHandlers) UIServiceTopology(resp http.ResponseWriter, req *http.Req return nil, err } - var err error - args.ServiceName, err = getPathSuffixUnescaped(req.URL.Path, "/v1/internal/ui/service-topology/") - if err != nil { - return nil, err - } + args.ServiceName = strings.TrimPrefix(req.URL.Path, "/v1/internal/ui/service-topology/") if args.ServiceName == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing service name"} } @@ -588,11 +576,7 @@ func (s *HTTPHandlers) UIGatewayIntentions(resp http.ResponseWriter, req *http.R } // Pull out the service name - var err error - name, err := getPathSuffixUnescaped(req.URL.Path, "/v1/internal/ui/gateway-intentions/") - if err != nil { - return nil, err - } + name := strings.TrimPrefix(req.URL.Path, "/v1/internal/ui/gateway-intentions/") if name == "" { return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing gateway name"} } @@ -674,10 +658,7 @@ func (s *HTTPHandlers) UIMetricsProxy(resp http.ResponseWriter, req *http.Reques // here. // Replace prefix in the path - subPath, err := getPathSuffixUnescaped(req.URL.Path, "/v1/internal/ui/metrics-proxy") - if err != nil { - return nil, err - } + subPath := strings.TrimPrefix(req.URL.Path, "/v1/internal/ui/metrics-proxy") // Append that to the BaseURL (which might contain a path prefix component) newURL := cfg.BaseURL + subPath