From ece75a03e308dfbf6f14c77c04098e2043d9fa6d Mon Sep 17 00:00:00 2001 From: "Chris S. Kim" Date: Tue, 25 Jan 2022 12:15:06 -0500 Subject: [PATCH] Remove incorrect usage of url.PathEscape (#12184) When r.toHTTP is called, http.Request is built with the path already escaped. This removes all calls to url.PathEscape that would have led to double-escaped URLs. --- api/acl.go | 34 +++++++++++++++++----------------- api/agent.go | 29 ++++++++++++++--------------- api/catalog.go | 11 +++++------ api/config_entry.go | 7 +++---- api/coordinate.go | 3 +-- api/discovery_chain.go | 3 +-- 6 files changed, 41 insertions(+), 46 deletions(-) diff --git a/api/acl.go b/api/acl.go index c3038e389..0f44494dd 100644 --- a/api/acl.go +++ b/api/acl.go @@ -552,7 +552,7 @@ func (a *ACL) Update(acl *ACLEntry, q *WriteOptions) (*WriteMeta, error) { // // Deprecated: Use TokenDelete instead. func (a *ACL) Destroy(id string, q *WriteOptions) (*WriteMeta, error) { - r := a.c.newRequest("PUT", "/v1/acl/destroy/"+url.PathEscape(id)) + r := a.c.newRequest("PUT", "/v1/acl/destroy/"+id) r.setWriteOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -571,7 +571,7 @@ func (a *ACL) Destroy(id string, q *WriteOptions) (*WriteMeta, error) { // // Deprecated: Use TokenClone instead. func (a *ACL) Clone(id string, q *WriteOptions) (string, *WriteMeta, error) { - r := a.c.newRequest("PUT", "/v1/acl/clone/"+url.PathEscape(id)) + r := a.c.newRequest("PUT", "/v1/acl/clone/"+id) r.setWriteOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -594,7 +594,7 @@ func (a *ACL) Clone(id string, q *WriteOptions) (string, *WriteMeta, error) { // // Deprecated: Use TokenRead instead. func (a *ACL) Info(id string, q *QueryOptions) (*ACLEntry, *QueryMeta, error) { - r := a.c.newRequest("GET", "/v1/acl/info/"+url.PathEscape(id)) + r := a.c.newRequest("GET", "/v1/acl/info/"+id) r.setQueryOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -696,7 +696,7 @@ func (a *ACL) TokenUpdate(token *ACLToken, q *WriteOptions) (*ACLToken, *WriteMe if token.AccessorID == "" { return nil, nil, fmt.Errorf("Must specify an AccessorID for Token Updating") } - r := a.c.newRequest("PUT", "/v1/acl/token/"+url.PathEscape(token.AccessorID)) + r := a.c.newRequest("PUT", "/v1/acl/token/"+token.AccessorID) r.setWriteOptions(q) r.obj = token rtt, resp, err := a.c.doRequest(r) @@ -725,7 +725,7 @@ func (a *ACL) TokenClone(tokenID string, description string, q *WriteOptions) (* return nil, nil, fmt.Errorf("Must specify a tokenID for Token Cloning") } - r := a.c.newRequest("PUT", "/v1/acl/token/"+url.PathEscape(tokenID)+"/clone") + r := a.c.newRequest("PUT", "/v1/acl/token/"+tokenID+"/clone") r.setWriteOptions(q) r.obj = struct{ Description string }{description} rtt, resp, err := a.c.doRequest(r) @@ -748,7 +748,7 @@ func (a *ACL) TokenClone(tokenID string, description string, q *WriteOptions) (* // TokenDelete removes a single ACL token. The tokenID parameter must be a valid // Accessor ID of an existing token. func (a *ACL) TokenDelete(tokenID string, q *WriteOptions) (*WriteMeta, error) { - r := a.c.newRequest("DELETE", "/v1/acl/token/"+url.PathEscape(tokenID)) + r := a.c.newRequest("DELETE", "/v1/acl/token/"+tokenID) r.setWriteOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -766,7 +766,7 @@ func (a *ACL) TokenDelete(tokenID string, q *WriteOptions) (*WriteMeta, error) { // TokenRead retrieves the full token details. The tokenID parameter must be a valid // Accessor ID of an existing token. func (a *ACL) TokenRead(tokenID string, q *QueryOptions) (*ACLToken, *QueryMeta, error) { - r := a.c.newRequest("GET", "/v1/acl/token/"+url.PathEscape(tokenID)) + r := a.c.newRequest("GET", "/v1/acl/token/"+tokenID) r.setQueryOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -871,7 +871,7 @@ func (a *ACL) PolicyUpdate(policy *ACLPolicy, q *WriteOptions) (*ACLPolicy, *Wri return nil, nil, fmt.Errorf("Must specify an ID in Policy Update") } - r := a.c.newRequest("PUT", "/v1/acl/policy/"+url.PathEscape(policy.ID)) + r := a.c.newRequest("PUT", "/v1/acl/policy/"+policy.ID) r.setWriteOptions(q) r.obj = policy rtt, resp, err := a.c.doRequest(r) @@ -893,7 +893,7 @@ func (a *ACL) PolicyUpdate(policy *ACLPolicy, q *WriteOptions) (*ACLPolicy, *Wri // PolicyDelete deletes a policy given its ID. func (a *ACL) PolicyDelete(policyID string, q *WriteOptions) (*WriteMeta, error) { - r := a.c.newRequest("DELETE", "/v1/acl/policy/"+url.PathEscape(policyID)) + r := a.c.newRequest("DELETE", "/v1/acl/policy/"+policyID) r.setWriteOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -910,7 +910,7 @@ func (a *ACL) PolicyDelete(policyID string, q *WriteOptions) (*WriteMeta, error) // PolicyRead retrieves the policy details including the rule set. func (a *ACL) PolicyRead(policyID string, q *QueryOptions) (*ACLPolicy, *QueryMeta, error) { - r := a.c.newRequest("GET", "/v1/acl/policy/"+url.PathEscape(policyID)) + r := a.c.newRequest("GET", "/v1/acl/policy/"+policyID) r.setQueryOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -1021,7 +1021,7 @@ func (a *ACL) RulesTranslate(rules io.Reader) (string, error) { // Deprecated: Support for the legacy syntax translation will be removed // when legacy ACL support is removed. func (a *ACL) RulesTranslateToken(tokenID string) (string, error) { - r := a.c.newRequest("GET", "/v1/acl/rules/translate/"+url.PathEscape(tokenID)) + r := a.c.newRequest("GET", "/v1/acl/rules/translate/"+tokenID) rtt, resp, err := a.c.doRequest(r) if err != nil { return "", err @@ -1076,7 +1076,7 @@ func (a *ACL) RoleUpdate(role *ACLRole, q *WriteOptions) (*ACLRole, *WriteMeta, return nil, nil, fmt.Errorf("Must specify an ID in Role Update") } - r := a.c.newRequest("PUT", "/v1/acl/role/"+url.PathEscape(role.ID)) + r := a.c.newRequest("PUT", "/v1/acl/role/"+role.ID) r.setWriteOptions(q) r.obj = role rtt, resp, err := a.c.doRequest(r) @@ -1098,7 +1098,7 @@ func (a *ACL) RoleUpdate(role *ACLRole, q *WriteOptions) (*ACLRole, *WriteMeta, // RoleDelete deletes a role given its ID. func (a *ACL) RoleDelete(roleID string, q *WriteOptions) (*WriteMeta, error) { - r := a.c.newRequest("DELETE", "/v1/acl/role/"+url.PathEscape(roleID)) + r := a.c.newRequest("DELETE", "/v1/acl/role/"+roleID) r.setWriteOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -1115,7 +1115,7 @@ func (a *ACL) RoleDelete(roleID string, q *WriteOptions) (*WriteMeta, error) { // RoleRead retrieves the role details (by ID). Returns nil if not found. func (a *ACL) RoleRead(roleID string, q *QueryOptions) (*ACLRole, *QueryMeta, error) { - r := a.c.newRequest("GET", "/v1/acl/role/"+url.PathEscape(roleID)) + r := a.c.newRequest("GET", "/v1/acl/role/"+roleID) r.setQueryOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -1365,7 +1365,7 @@ func (a *ACL) BindingRuleUpdate(rule *ACLBindingRule, q *WriteOptions) (*ACLBind return nil, nil, fmt.Errorf("Must specify an ID in Binding Rule Update") } - r := a.c.newRequest("PUT", "/v1/acl/binding-rule/"+url.PathEscape(rule.ID)) + r := a.c.newRequest("PUT", "/v1/acl/binding-rule/"+rule.ID) r.setWriteOptions(q) r.obj = rule rtt, resp, err := a.c.doRequest(r) @@ -1387,7 +1387,7 @@ func (a *ACL) BindingRuleUpdate(rule *ACLBindingRule, q *WriteOptions) (*ACLBind // BindingRuleDelete deletes a binding rule given its ID. func (a *ACL) BindingRuleDelete(bindingRuleID string, q *WriteOptions) (*WriteMeta, error) { - r := a.c.newRequest("DELETE", "/v1/acl/binding-rule/"+url.PathEscape(bindingRuleID)) + r := a.c.newRequest("DELETE", "/v1/acl/binding-rule/"+bindingRuleID) r.setWriteOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -1404,7 +1404,7 @@ func (a *ACL) BindingRuleDelete(bindingRuleID string, q *WriteOptions) (*WriteMe // BindingRuleRead retrieves the binding rule details. Returns nil if not found. func (a *ACL) BindingRuleRead(bindingRuleID string, q *QueryOptions) (*ACLBindingRule, *QueryMeta, error) { - r := a.c.newRequest("GET", "/v1/acl/binding-rule/"+url.PathEscape(bindingRuleID)) + r := a.c.newRequest("GET", "/v1/acl/binding-rule/"+bindingRuleID) r.setQueryOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { diff --git a/api/agent.go b/api/agent.go index 04dd059d9..7bbe39ea7 100644 --- a/api/agent.go +++ b/api/agent.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "net/http" - "net/url" ) // ServiceKind is the kind of service being registered. @@ -628,7 +627,7 @@ func (a *Agent) AgentHealthServiceByID(serviceID string) (string, *AgentServiceC } func (a *Agent) AgentHealthServiceByIDOpts(serviceID string, q *QueryOptions) (string, *AgentServiceChecksInfo, error) { - path := fmt.Sprintf("/v1/agent/health/service/id/%v", url.PathEscape(serviceID)) + path := fmt.Sprintf("/v1/agent/health/service/id/%v", serviceID) r := a.c.newRequest("GET", path) r.setQueryOptions(q) r.params.Add("format", "json") @@ -669,7 +668,7 @@ func (a *Agent) AgentHealthServiceByName(service string) (string, []AgentService } func (a *Agent) AgentHealthServiceByNameOpts(service string, q *QueryOptions) (string, []AgentServiceChecksInfo, error) { - path := fmt.Sprintf("/v1/agent/health/service/name/%v", url.PathEscape(service)) + path := fmt.Sprintf("/v1/agent/health/service/name/%v", service) r := a.c.newRequest("GET", path) r.setQueryOptions(q) r.params.Add("format", "json") @@ -707,7 +706,7 @@ func (a *Agent) AgentHealthServiceByNameOpts(service string, q *QueryOptions) (s // agent-local state. That means there is no persistent raft index so we block // based on object hash instead. func (a *Agent) Service(serviceID string, q *QueryOptions) (*AgentService, *QueryMeta, error) { - r := a.c.newRequest("GET", "/v1/agent/service/"+url.PathEscape(serviceID)) + r := a.c.newRequest("GET", "/v1/agent/service/"+serviceID) r.setQueryOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -812,7 +811,7 @@ func (a *Agent) serviceRegister(service *AgentServiceRegistration, opts ServiceR // ServiceDeregister is used to deregister a service with // the local agent func (a *Agent) ServiceDeregister(serviceID string) error { - r := a.c.newRequest("PUT", "/v1/agent/service/deregister/"+url.PathEscape(serviceID)) + r := a.c.newRequest("PUT", "/v1/agent/service/deregister/"+serviceID) _, resp, err := a.c.doRequest(r) if err != nil { return err @@ -827,7 +826,7 @@ func (a *Agent) ServiceDeregister(serviceID string) error { // ServiceDeregisterOpts is used to deregister a service with // the local agent with QueryOptions. func (a *Agent) ServiceDeregisterOpts(serviceID string, q *QueryOptions) error { - r := a.c.newRequest("PUT", "/v1/agent/service/deregister/"+url.PathEscape(serviceID)) + r := a.c.newRequest("PUT", "/v1/agent/service/deregister/"+serviceID) r.setQueryOptions(q) _, resp, err := a.c.doRequest(r) if err != nil { @@ -884,7 +883,7 @@ func (a *Agent) updateTTL(checkID, note, status string) error { default: return fmt.Errorf("Invalid status: %s", status) } - endpoint := fmt.Sprintf("/v1/agent/check/%s/%s", url.PathEscape(status), url.PathEscape(checkID)) + endpoint := fmt.Sprintf("/v1/agent/check/%s/%s", status, checkID) r := a.c.newRequest("PUT", endpoint) r.params.Set("note", note) _, resp, err := a.c.doRequest(r) @@ -932,7 +931,7 @@ func (a *Agent) UpdateTTLOpts(checkID, output, status string, q *QueryOptions) e return fmt.Errorf("Invalid status: %s", status) } - endpoint := fmt.Sprintf("/v1/agent/check/update/%s", url.PathEscape(checkID)) + endpoint := fmt.Sprintf("/v1/agent/check/update/%s", checkID) r := a.c.newRequest("PUT", endpoint) r.setQueryOptions(q) r.obj = &checkUpdate{ @@ -976,7 +975,7 @@ func (a *Agent) CheckDeregister(checkID string) error { // CheckDeregisterOpts is used to deregister a check with // the local agent using query options func (a *Agent) CheckDeregisterOpts(checkID string, q *QueryOptions) error { - r := a.c.newRequest("PUT", "/v1/agent/check/deregister/"+url.PathEscape(checkID)) + r := a.c.newRequest("PUT", "/v1/agent/check/deregister/"+checkID) r.setQueryOptions(q) _, resp, err := a.c.doRequest(r) if err != nil { @@ -992,7 +991,7 @@ func (a *Agent) CheckDeregisterOpts(checkID string, q *QueryOptions) error { // Join is used to instruct the agent to attempt a join to // another cluster member func (a *Agent) Join(addr string, wan bool) error { - r := a.c.newRequest("PUT", "/v1/agent/join/"+url.PathEscape(addr)) + r := a.c.newRequest("PUT", "/v1/agent/join/"+addr) if wan { r.params.Set("wan", "1") } @@ -1044,7 +1043,7 @@ func (a *Agent) ForceLeavePrune(node string) error { // ForceLeaveOpts is used to have the agent eject a failed node or remove it // completely from the list of members. func (a *Agent) ForceLeaveOpts(node string, opts ForceLeaveOpts) error { - r := a.c.newRequest("PUT", "/v1/agent/force-leave/"+url.PathEscape(node)) + r := a.c.newRequest("PUT", "/v1/agent/force-leave/"+node) if opts.Prune { r.params.Set("prune", "1") } @@ -1108,7 +1107,7 @@ func (a *Agent) ConnectCARoots(q *QueryOptions) (*CARootList, *QueryMeta, error) // ConnectCALeaf gets the leaf certificate for the given service ID. func (a *Agent) ConnectCALeaf(serviceID string, q *QueryOptions) (*LeafCert, *QueryMeta, error) { - r := a.c.newRequest("GET", "/v1/agent/connect/ca/leaf/"+url.PathEscape(serviceID)) + r := a.c.newRequest("GET", "/v1/agent/connect/ca/leaf/"+serviceID) r.setQueryOptions(q) rtt, resp, err := a.c.doRequest(r) if err != nil { @@ -1136,7 +1135,7 @@ func (a *Agent) EnableServiceMaintenance(serviceID, reason string) error { } func (a *Agent) EnableServiceMaintenanceOpts(serviceID, reason string, q *QueryOptions) error { - r := a.c.newRequest("PUT", "/v1/agent/service/maintenance/"+url.PathEscape(serviceID)) + r := a.c.newRequest("PUT", "/v1/agent/service/maintenance/"+serviceID) r.setQueryOptions(q) r.params.Set("enable", "true") r.params.Set("reason", reason) @@ -1158,7 +1157,7 @@ func (a *Agent) DisableServiceMaintenance(serviceID string) error { } func (a *Agent) DisableServiceMaintenanceOpts(serviceID string, q *QueryOptions) error { - r := a.c.newRequest("PUT", "/v1/agent/service/maintenance/"+url.PathEscape(serviceID)) + r := a.c.newRequest("PUT", "/v1/agent/service/maintenance/"+serviceID) r.setQueryOptions(q) r.params.Set("enable", "false") _, resp, err := a.c.doRequest(r) @@ -1355,7 +1354,7 @@ func (a *Agent) updateTokenFallback(token string, q *WriteOptions, targets ...st } func (a *Agent) updateTokenOnce(target, token string, q *WriteOptions) (*WriteMeta, int, error) { - r := a.c.newRequest("PUT", fmt.Sprintf("/v1/agent/token/%s", url.PathEscape(target))) + r := a.c.newRequest("PUT", fmt.Sprintf("/v1/agent/token/%s", target)) r.setWriteOptions(q) r.obj = &AgentToken{Token: token} diff --git a/api/catalog.go b/api/catalog.go index 54fcce85f..80ae325ea 100644 --- a/api/catalog.go +++ b/api/catalog.go @@ -2,7 +2,6 @@ package api import ( "net" - "net/url" "strconv" ) @@ -255,9 +254,9 @@ func (c *Catalog) ConnectMultipleTags(service string, tags []string, q *QueryOpt } func (c *Catalog) service(service string, tags []string, q *QueryOptions, connect bool) ([]*CatalogService, *QueryMeta, error) { - path := "/v1/catalog/service/" + url.PathEscape(service) + path := "/v1/catalog/service/" + service if connect { - path = "/v1/catalog/connect/" + url.PathEscape(service) + path = "/v1/catalog/connect/" + service } r := c.c.newRequest("GET", path) r.setQueryOptions(q) @@ -288,7 +287,7 @@ func (c *Catalog) service(service string, tags []string, q *QueryOptions, connec // Node is used to query for service information about a single node func (c *Catalog) Node(node string, q *QueryOptions) (*CatalogNode, *QueryMeta, error) { - r := c.c.newRequest("GET", "/v1/catalog/node/"+url.PathEscape(node)) + r := c.c.newRequest("GET", "/v1/catalog/node/"+node) r.setQueryOptions(q) rtt, resp, err := c.c.doRequest(r) if err != nil { @@ -315,7 +314,7 @@ func (c *Catalog) Node(node string, q *QueryOptions) (*CatalogNode, *QueryMeta, // a map of service ids to services. This different structure allows for using the wildcard specifier // '*' for the Namespace in the QueryOptions. func (c *Catalog) NodeServiceList(node string, q *QueryOptions) (*CatalogNodeServiceList, *QueryMeta, error) { - r := c.c.newRequest("GET", "/v1/catalog/node-services/"+url.PathEscape(node)) + r := c.c.newRequest("GET", "/v1/catalog/node-services/"+node) r.setQueryOptions(q) rtt, resp, err := c.c.doRequest(r) if err != nil { @@ -339,7 +338,7 @@ func (c *Catalog) NodeServiceList(node string, q *QueryOptions) (*CatalogNodeSer // GatewayServices is used to query the services associated with an ingress gateway or terminating gateway. func (c *Catalog) GatewayServices(gateway string, q *QueryOptions) ([]*GatewayService, *QueryMeta, error) { - r := c.c.newRequest("GET", "/v1/catalog/gateway-services/"+url.PathEscape(gateway)) + r := c.c.newRequest("GET", "/v1/catalog/gateway-services/"+gateway) r.setQueryOptions(q) rtt, resp, err := c.c.doRequest(r) if err != nil { diff --git a/api/config_entry.go b/api/config_entry.go index f1c4091d1..91c407bb5 100644 --- a/api/config_entry.go +++ b/api/config_entry.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "io" - "net/url" "strconv" "strings" "time" @@ -379,7 +378,7 @@ func (conf *ConfigEntries) Get(kind string, name string, q *QueryOptions) (Confi return nil, nil, err } - r := conf.c.newRequest("GET", fmt.Sprintf("/v1/config/%s/%s", url.PathEscape(kind), url.PathEscape(name))) + r := conf.c.newRequest("GET", fmt.Sprintf("/v1/config/%s/%s", kind, name)) r.setQueryOptions(q) rtt, resp, err := conf.c.doRequest(r) if err != nil { @@ -406,7 +405,7 @@ func (conf *ConfigEntries) List(kind string, q *QueryOptions) ([]ConfigEntry, *Q return nil, nil, fmt.Errorf("The kind parameter must not be empty") } - r := conf.c.newRequest("GET", fmt.Sprintf("/v1/config/%s", url.PathEscape(kind))) + r := conf.c.newRequest("GET", fmt.Sprintf("/v1/config/%s", kind)) r.setQueryOptions(q) rtt, resp, err := conf.c.doRequest(r) if err != nil { @@ -486,7 +485,7 @@ func (conf *ConfigEntries) delete(kind, name string, params map[string]string, w return false, nil, fmt.Errorf("Both kind and name parameters must not be empty") } - r := conf.c.newRequest("DELETE", fmt.Sprintf("/v1/config/%s/%s", url.PathEscape(kind), url.PathEscape(name))) + r := conf.c.newRequest("DELETE", fmt.Sprintf("/v1/config/%s/%s", kind, name)) r.setWriteOptions(w) for param, value := range params { r.params.Set(param, value) diff --git a/api/coordinate.go b/api/coordinate.go index bd607e32f..7ef6ce274 100644 --- a/api/coordinate.go +++ b/api/coordinate.go @@ -2,7 +2,6 @@ package api import ( "github.com/hashicorp/serf/coordinate" - "net/url" ) // CoordinateEntry represents a node and its associated network coordinate. @@ -97,7 +96,7 @@ func (c *Coordinate) Update(coord *CoordinateEntry, q *WriteOptions) (*WriteMeta // Node is used to return the coordinates of a single node in the LAN pool. func (c *Coordinate) Node(node string, q *QueryOptions) ([]*CoordinateEntry, *QueryMeta, error) { - r := c.c.newRequest("GET", "/v1/coordinate/node/"+url.PathEscape(node)) + r := c.c.newRequest("GET", "/v1/coordinate/node/"+node) r.setQueryOptions(q) rtt, resp, err := c.c.doRequest(r) if err != nil { diff --git a/api/discovery_chain.go b/api/discovery_chain.go index 10150f5de..29bda8591 100644 --- a/api/discovery_chain.go +++ b/api/discovery_chain.go @@ -3,7 +3,6 @@ package api import ( "encoding/json" "fmt" - "net/url" "time" ) @@ -27,7 +26,7 @@ func (d *DiscoveryChain) Get(name string, opts *DiscoveryChainOptions, q *QueryO method = "POST" } - r := d.c.newRequest(method, fmt.Sprintf("/v1/discovery-chain/%s", url.PathEscape(name))) + r := d.c.newRequest(method, fmt.Sprintf("/v1/discovery-chain/%s", name)) r.setQueryOptions(q) if opts != nil {