fix: stale querystring parameter value as boolean (#15605)

* Add changes to make stale querystring param boolean Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> * Make error message more consistent Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> * Changes from code review + Adding CHANGELOG file Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> * Changes from code review to use github.com/shoenig/test package Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> * Change must.Nil() to must.NoError() Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> * Minor fix on the import order Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> * Fix existing code format too Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> * Minor changes addressing code review feedbacks Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> * swap must.EqOp() order of param provided Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg> Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
hashicorp · Jan 23, 2023 · 839ae94 · 839ae94
1 parent c38dcb8
commit 839ae94
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
@@ -783,10 +783,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] == ""
 	}
 }
 
@@ -884,7 +890,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) {