From 03b683f7025e4b9104a4516fd5ff77d77ae3d78d Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Wed, 27 Jun 2018 07:40:06 +0200
Subject: [PATCH] agent: 400 error on invalid UUID format, api handles errors
 properly

---
 agent/intentions_endpoint.go      |  8 ++++++++
 agent/intentions_endpoint_test.go | 17 +++++++++++++++++
 api/connect_intention.go          |  7 ++++++-
 api/connect_intention_test.go     | 16 ++++++++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/agent/intentions_endpoint.go b/agent/intentions_endpoint.go
index 642c7da7b..d4d6736ab 100644
--- a/agent/intentions_endpoint.go
+++ b/agent/intentions_endpoint.go
@@ -212,6 +212,14 @@ func (s *HTTPServer) IntentionSpecificGet(id string, resp http.ResponseWriter, r
 			return nil, nil
 		}
 
+		// Not ideal, but there are a number of error scenarios that are not
+		// user error (400). We look for a specific case of invalid UUID
+		// to detect a parameter error and return a 400 response. The error
+		// is not a constant type or message, so we have to use strings.Contains
+		if strings.Contains(err.Error(), "UUID") {
+			return nil, BadRequestError{Reason: err.Error()}
+		}
+
 		return nil, err
 	}
 
diff --git a/agent/intentions_endpoint_test.go b/agent/intentions_endpoint_test.go
index 4eb6c985c..b800b20ac 100644
--- a/agent/intentions_endpoint_test.go
+++ b/agent/intentions_endpoint_test.go
@@ -355,6 +355,23 @@ func TestIntentionsSpecificGet_good(t *testing.T) {
 	assert.Equal(ixn, value)
 }
 
+func TestIntentionsSpecificGet_invalidId(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	a := NewTestAgent(t.Name(), "")
+	defer a.Shutdown()
+
+	// Read intention with bad ID
+	req, _ := http.NewRequest("GET", "/v1/connect/intentions/hello", nil)
+	resp := httptest.NewRecorder()
+	obj, err := a.srv.IntentionSpecific(resp, req)
+	require.Nil(obj)
+	require.Error(err)
+	require.IsType(BadRequestError{}, err)
+	require.Contains(err.Error(), "UUID")
+}
+
 func TestIntentionsSpecificUpdate_good(t *testing.T) {
 	t.Parallel()
 
diff --git a/api/connect_intention.go b/api/connect_intention.go
index d171d2289..a996c03e5 100644
--- a/api/connect_intention.go
+++ b/api/connect_intention.go
@@ -1,7 +1,9 @@
 package api
 
 import (
+	"bytes"
 	"fmt"
+	"io"
 	"time"
 )
 
@@ -172,7 +174,10 @@ func (h *Connect) IntentionGet(id string, q *QueryOptions) (*Intention, *QueryMe
 	if resp.StatusCode == 404 {
 		return nil, qm, nil
 	} else if resp.StatusCode != 200 {
-		return nil, nil, fmt.Errorf("Unexpected response code: %d", resp.StatusCode)
+		var buf bytes.Buffer
+		io.Copy(&buf, resp.Body)
+		return nil, nil, fmt.Errorf(
+			"Unexpected response %d: %s", resp.StatusCode, buf.String())
 	}
 
 	var out Intention
diff --git a/api/connect_intention_test.go b/api/connect_intention_test.go
index bec1d8ef2..0ace2afda 100644
--- a/api/connect_intention_test.go
+++ b/api/connect_intention_test.go
@@ -61,6 +61,22 @@ func TestAPI_ConnectIntentionCreateListGetUpdateDelete(t *testing.T) {
 	require.Nil(actual)
 }
 
+func TestAPI_ConnectIntentionGet_invalidId(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	c, s := makeClient(t)
+	defer s.Stop()
+
+	connect := c.Connect()
+
+	// Get it
+	actual, _, err := connect.IntentionGet("hello", nil)
+	require.Nil(actual)
+	require.Error(err)
+	require.Contains(err.Error(), "UUID") // verify it contains the message
+}
+
 func TestAPI_ConnectIntentionMatch(t *testing.T) {
 	t.Parallel()