Backport of fix: stale querystring parameter value as boolean into …

…release/1.4.x (#15651) This pull request was automerged via backport-assistant
hashicorp · Jan 3, 2023 · e1dc8f0 · e1dc8f0
1 parent c249fdd
commit e1dc8f0
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 21 deletions.
diff --git a/.changelog/15605.txt b/.changelog/15605.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+api: Fix stale querystring parameter value as boolean
+```
diff --git a/command/agent/http.go b/command/agent/http.go
@@ -773,10 +773,16 @@ func parseWait(resp http.ResponseWriter, req *http.Request, b *structs.QueryOpti
 }
 
 // parseConsistency is used to parse the ?stale query params.
-func parseConsistency(req *http.Request, b *structs.QueryOptions) {
+func parseConsistency(resp http.ResponseWriter, req *http.Request, b *structs.QueryOptions) {
 	query := req.URL.Query()
-	if _, ok := query["stale"]; ok {
-		b.AllowStale = true
+	if staleVal, ok := query["stale"]; ok {
+		staleQuery, err := strconv.ParseBool(staleVal[0])
+		if err != nil {
+			resp.WriteHeader(400)
+			resp.Write([]byte(fmt.Sprintf("Expect `true` or `false` for `stale` query string parameter, got %s", staleVal[0])))
+		}
+
+		b.AllowStale = staleQuery || staleVal[0] == ""
 	}
 }
 
@@ -874,7 +880,7 @@ func (s *HTTPServer) parseToken(req *http.Request, token *string) {
 func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, r *string, b *structs.QueryOptions) bool {
 	s.parseRegion(req, r)
 	s.parseToken(req, &b.AuthToken)
-	parseConsistency(req, b)
+	parseConsistency(resp, req, b)
 	parsePrefix(req, b)
 	parseNamespace(req, &b.Namespace)
 	parsePagination(req, b)

diff --git a/command/agent/http_test.go b/command/agent/http_test.go
@@ -29,6 +29,7 @@ import (
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/hashicorp/nomad/testutil"
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -476,29 +477,37 @@ func TestParseWait_InvalidIndex(t *testing.T) {
 func TestParseConsistency(t *testing.T) {
 	ci.Parallel(t)
 	var b structs.QueryOptions
+	var resp *httptest.ResponseRecorder
 
-	req, err := http.NewRequest("GET",
-		"/v1/catalog/nodes?stale", nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
+	testCases := [2]string{"/v1/catalog/nodes?stale", "/v1/catalog/nodes?stale=true"}
+	for _, urlPath := range testCases {
+		req, err := http.NewRequest("GET", urlPath, nil)
+		must.NoError(t, err)
+		resp = httptest.NewRecorder()
+		parseConsistency(resp, req, &b)
+		must.True(t, b.AllowStale)
 	}
 
-	parseConsistency(req, &b)
-	if !b.AllowStale {
-		t.Fatalf("Bad: %v", b)
-	}
+	req, err := http.NewRequest("GET", "/v1/catalog/nodes?stale=false", nil)
+	must.NoError(t, err)
+	resp = httptest.NewRecorder()
+	parseConsistency(resp, req, &b)
+	must.False(t, b.AllowStale)
+
+	req, err = http.NewRequest("GET", "/v1/catalog/nodes?stale=random", nil)
+	must.NoError(t, err)
+	resp = httptest.NewRecorder()
+	parseConsistency(resp, req, &b)
+	must.False(t, b.AllowStale)
+	must.EqOp(t, 400, resp.Code)
 
 	b = structs.QueryOptions{}
-	req, err = http.NewRequest("GET",
-		"/v1/catalog/nodes?consistent", nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	req, err = http.NewRequest("GET", "/v1/catalog/nodes?consistent", nil)
+	must.NoError(t, err)
 
-	parseConsistency(req, &b)
-	if b.AllowStale {
-		t.Fatalf("Bad: %v", b)
-	}
+	resp = httptest.NewRecorder()
+	parseConsistency(resp, req, &b)
+	must.False(t, b.AllowStale)
 }
 
 func TestParseRegion(t *testing.T) {