Merge pull request #2336 from hashicorp/b-fix-api-panic

Fix API panic and bad missing port check

api/api.go

@@ -374,9 +374,12 @@ func (r *request) toHTTP() (*http.Request, error) {
 }
 
 // newRequest is used to create a new request
-func (c *Client) newRequest(method, path string) *request {
+func (c *Client) newRequest(method, path string) (*request, error) {
 	base, _ := url.Parse(c.config.Address)
-	u, _ := url.Parse(path)
+	u, err := url.Parse(path)
+	if err != nil {
+		return nil, err
+	}
 	r := &request{
 		config: &c.config,
 		method: method,
@@ -402,7 +405,7 @@ func (c *Client) newRequest(method, path string) *request {
 		}
 	}
 
-	return r
+	return r, nil
 }
 
 // multiCloser is to wrap a ReadCloser such that when close is called, multiple
@@ -463,7 +466,10 @@ func (c *Client) doRequest(r *request) (time.Duration, *http.Response, error) {
 // rawQuery makes a GET request to the specified endpoint but returns just the
 // response body.
 func (c *Client) rawQuery(endpoint string, q *QueryOptions) (io.ReadCloser, error) {
-	r := c.newRequest("GET", endpoint)
+	r, err := c.newRequest("GET", endpoint)
+	if err != nil {
+		return nil, err
+	}
 	r.setQueryOptions(q)
 	_, resp, err := requireOK(c.doRequest(r))
 	if err != nil {
@@ -477,7 +483,10 @@ func (c *Client) rawQuery(endpoint string, q *QueryOptions) (io.ReadCloser, erro
 // and deserialize the response into an interface using
 // standard Nomad conventions.
 func (c *Client) query(endpoint string, out interface{}, q *QueryOptions) (*QueryMeta, error) {
-	r := c.newRequest("GET", endpoint)
+	r, err := c.newRequest("GET", endpoint)
+	if err != nil {
+		return nil, err
+	}
 	r.setQueryOptions(q)
 	rtt, resp, err := requireOK(c.doRequest(r))
 	if err != nil {
@@ -498,7 +507,10 @@ func (c *Client) query(endpoint string, out interface{}, q *QueryOptions) (*Quer
 // write is used to do a PUT request against an endpoint
 // and serialize/deserialized using the standard Nomad conventions.
 func (c *Client) write(endpoint string, in, out interface{}, q *WriteOptions) (*WriteMeta, error) {
-	r := c.newRequest("PUT", endpoint)
+	r, err := c.newRequest("PUT", endpoint)
+	if err != nil {
+		return nil, err
+	}
 	r.setWriteOptions(q)
 	r.obj = in
 	rtt, resp, err := requireOK(c.doRequest(r))
@@ -518,10 +530,13 @@ func (c *Client) write(endpoint string, in, out interface{}, q *WriteOptions) (*
 	return wm, nil
 }
 
-// write is used to do a PUT request against an endpoint
+// delete is used to do a DELETE request against an endpoint
 // and serialize/deserialized using the standard Nomad conventions.
 func (c *Client) delete(endpoint string, out interface{}, q *WriteOptions) (*WriteMeta, error) {
-	r := c.newRequest("DELETE", endpoint)
+	r, err := c.newRequest("DELETE", endpoint)
+	if err != nil {
+		return nil, err
+	}
 	r.setWriteOptions(q)
 	rtt, resp, err := requireOK(c.doRequest(r))
 	if err != nil {

api/api_test.go

@@ -118,7 +118,7 @@ func TestSetQueryOptions(t *testing.T) {
 	c, s := makeClient(t, nil, nil)
 	defer s.Stop()
 
-	r := c.newRequest("GET", "/v1/jobs")
+	r, _ := c.newRequest("GET", "/v1/jobs")
 	q := &QueryOptions{
 		Region:     "foo",
 		AllowStale: true,
@@ -145,7 +145,7 @@ func TestSetWriteOptions(t *testing.T) {
 	c, s := makeClient(t, nil, nil)
 	defer s.Stop()
 
-	r := c.newRequest("GET", "/v1/jobs")
+	r, _ := c.newRequest("GET", "/v1/jobs")
 	q := &WriteOptions{
 		Region: "foo",
 	}
@@ -160,7 +160,7 @@ func TestRequestToHTTP(t *testing.T) {
 	c, s := makeClient(t, nil, nil)
 	defer s.Stop()
 
-	r := c.newRequest("DELETE", "/v1/jobs/foo")
+	r, _ := c.newRequest("DELETE", "/v1/jobs/foo")
 	q := &QueryOptions{
 		Region: "foo",
 	}
@@ -222,7 +222,7 @@ func TestQueryString(t *testing.T) {
 	c, s := makeClient(t, nil, nil)
 	defer s.Stop()
 
-	r := c.newRequest("PUT", "/v1/abc?foo=bar&baz=zip")
+	r, _ := c.newRequest("PUT", "/v1/abc?foo=bar&baz=zip")
 	q := &WriteOptions{Region: "foo"}
 	r.setWriteOptions(q)
 

api/operator.go

@@ -45,7 +45,10 @@ type RaftConfiguration struct {
 
 // RaftGetConfiguration is used to query the current Raft peer set.
 func (op *Operator) RaftGetConfiguration(q *QueryOptions) (*RaftConfiguration, error) {
-	r := op.c.newRequest("GET", "/v1/operator/raft/configuration")
+	r, err := op.c.newRequest("GET", "/v1/operator/raft/configuration")
+	if err != nil {
+		return nil, err
+	}
 	r.setQueryOptions(q)
 	_, resp, err := requireOK(op.c.doRequest(r))
 	if err != nil {
@@ -64,7 +67,10 @@ func (op *Operator) RaftGetConfiguration(q *QueryOptions) (*RaftConfiguration, e
 // quorum but no longer known to Serf or the catalog) by address in the form of
 // "IP:port".
 func (op *Operator) RaftRemovePeerByAddress(address string, q *WriteOptions) error {
-	r := op.c.newRequest("DELETE", "/v1/operator/raft/peer")
+	r, err := op.c.newRequest("DELETE", "/v1/operator/raft/peer")
+	if err != nil {
+		return err
+	}
 	r.setWriteOptions(q)
 
 	// TODO (alexdadgar) Currently we made address a query parameter. Once