From 051d3c8b07806e1ced048349c41a369f36fb77d3 Mon Sep 17 00:00:00 2001
From: Zachary Wasserman <zachwass2000@gmail.com>
Date: Tue, 20 Nov 2018 17:19:24 -0800
Subject: [PATCH] Fix errors when trying to retrieve specs with spaces in name
 (#1957)

We need to properly escape and unescape the name parameter.

Fixes #1948
---
 server/service/client_labels.go  | 4 ++--
 server/service/client_packs.go   | 4 ++--
 server/service/client_queries.go | 4 ++--
 server/service/transport.go      | 9 +++++++--
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/server/service/client_labels.go b/server/service/client_labels.go
index 4cdba3618d..3df60610c4 100644
--- a/server/service/client_labels.go
+++ b/server/service/client_labels.go
@@ -42,7 +42,7 @@ func (c *Client) ApplyLabels(specs []*kolide.LabelSpec) error {
 
 // GetLabel retrieves information about a label by name
 func (c *Client) GetLabel(name string) (*kolide.LabelSpec, error) {
-	verb, path := "GET", "/api/v1/kolide/spec/labels/"+url.QueryEscape(name)
+	verb, path := "GET", "/api/v1/kolide/spec/labels/"+url.PathEscape(name)
 	response, err := c.AuthenticatedDo(verb, path, nil)
 	if err != nil {
 		return nil, errors.Wrap(err, "GET /api/v1/kolide/spec/labels")
@@ -105,7 +105,7 @@ func (c *Client) GetLabels() ([]*kolide.LabelSpec, error) {
 
 // DeleteLabel deletes the label with the matching name.
 func (c *Client) DeleteLabel(name string) error {
-	verb, path := "DELETE", "/api/v1/kolide/labels/"+url.QueryEscape(name)
+	verb, path := "DELETE", "/api/v1/kolide/labels/"+url.PathEscape(name)
 	response, err := c.AuthenticatedDo(verb, path, nil)
 	if err != nil {
 		return errors.Wrapf(err, "%s %s", verb, path)
diff --git a/server/service/client_packs.go b/server/service/client_packs.go
index 35791f9ae6..f8bc253bbd 100644
--- a/server/service/client_packs.go
+++ b/server/service/client_packs.go
@@ -42,7 +42,7 @@ func (c *Client) ApplyPacks(specs []*kolide.PackSpec) error {
 
 // GetPack retrieves information about a pack
 func (c *Client) GetPack(name string) (*kolide.PackSpec, error) {
-	verb, path := "GET", "/api/v1/kolide/spec/packs/"+url.QueryEscape(name)
+	verb, path := "GET", "/api/v1/kolide/spec/packs/"+url.PathEscape(name)
 	response, err := c.AuthenticatedDo(verb, path, nil)
 	if err != nil {
 		return nil, errors.Wrap(err, "GET /api/v1/kolide/spec/packs")
@@ -105,7 +105,7 @@ func (c *Client) GetPacks() ([]*kolide.PackSpec, error) {
 
 // DeletePack deletes the pack with the matching name.
 func (c *Client) DeletePack(name string) error {
-	verb, path := "DELETE", "/api/v1/kolide/packs/"+url.QueryEscape(name)
+	verb, path := "DELETE", "/api/v1/kolide/packs/"+url.PathEscape(name)
 	response, err := c.AuthenticatedDo(verb, path, nil)
 	if err != nil {
 		return errors.Wrapf(err, "%s %s", verb, path)
diff --git a/server/service/client_queries.go b/server/service/client_queries.go
index e72d7f45cc..721f56b5c7 100644
--- a/server/service/client_queries.go
+++ b/server/service/client_queries.go
@@ -42,7 +42,7 @@ func (c *Client) ApplyQueries(specs []*kolide.QuerySpec) error {
 
 // GetQuery retrieves the list of all Queries.
 func (c *Client) GetQuery(name string) (*kolide.QuerySpec, error) {
-	verb, path := "GET", "/api/v1/kolide/spec/queries/"+url.QueryEscape(name)
+	verb, path := "GET", "/api/v1/kolide/spec/queries/"+url.PathEscape(name)
 	response, err := c.AuthenticatedDo(verb, path, nil)
 	if err != nil {
 		return nil, errors.Wrapf(err, "%s %s", verb, path)
@@ -105,7 +105,7 @@ func (c *Client) GetQueries() ([]*kolide.QuerySpec, error) {
 
 // DeleteQuery deletes the query with the matching name.
 func (c *Client) DeleteQuery(name string) error {
-	verb, path := "DELETE", "/api/v1/kolide/queries/"+url.QueryEscape(name)
+	verb, path := "DELETE", "/api/v1/kolide/queries/"+url.PathEscape(name)
 	response, err := c.AuthenticatedDo(verb, path, nil)
 	if err != nil {
 		return errors.Wrapf(err, "%s %s", verb, path)
diff --git a/server/service/transport.go b/server/service/transport.go
index edec258230..bc47d9bdd9 100644
--- a/server/service/transport.go
+++ b/server/service/transport.go
@@ -3,13 +3,14 @@ package service
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"io"
 	"net/http"
+	"net/url"
 	"strconv"
 
 	"github.com/gorilla/mux"
 	"github.com/kolide/fleet/server/kolide"
+	"github.com/pkg/errors"
 )
 
 var (
@@ -74,7 +75,11 @@ func nameFromRequest(r *http.Request, varName string) (string, error) {
 	if !ok {
 		return "", errBadRoute
 	}
-	return name, nil
+	unescaped, err := url.PathUnescape(name)
+	if err != nil {
+		return "", errors.Wrap(err, "unescape name in path")
+	}
+	return unescaped, nil
 }
 
 // default number of items to include per page