Skip to content

Commit

Permalink
command: fixup parsing of stale query parameter (#15631)
Browse files Browse the repository at this point in the history
In #15605 we fixed the bug where the presense of "stale" query parameter
was mean to imply stale, even if the value of the parameter was "false"
or malformed. In parsing, we missed the case where the slice of values
would be nil which lead to a failing test case that was missed because
CI didn't run against the original PR.
  • Loading branch information
shoenig committed Jan 3, 2023
1 parent 76e1856 commit e2f9120
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
10 changes: 7 additions & 3 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -786,13 +786,17 @@ func parseWait(resp http.ResponseWriter, req *http.Request, b *structs.QueryOpti
func parseConsistency(resp http.ResponseWriter, req *http.Request, b *structs.QueryOptions) {
query := req.URL.Query()
if staleVal, ok := query["stale"]; ok {
if len(staleVal) == 0 || staleVal[0] == "" {
b.AllowStale = true
return
}
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])))
_, _ = resp.Write([]byte(fmt.Sprintf("Expect `true` or `false` for `stale` query string parameter, got %s", staleVal[0])))
return
}

b.AllowStale = staleQuery || staleVal[0] == ""
b.AllowStale = staleQuery
}
}

Expand Down
4 changes: 2 additions & 2 deletions command/operator_debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func runTestCases(t *testing.T, cases testCases) {
out := ui.OutputWriter.String()
outerr := ui.ErrorWriter.String()

assert.Equalf(t, code, c.expectedCode, "did not get expected exit code")
assert.Equalf(t, c.expectedCode, code, "did not get expected exit code")

if len(c.expectedOutputs) > 0 {
if assert.NotEmpty(t, out, "command output was empty") {
Expand Down Expand Up @@ -727,7 +727,7 @@ func TestDebug_CollectConsul(t *testing.T) {

// Create an embedded Consul server
testconsul, err := consultest.NewTestServerConfigT(t, func(c *consultest.TestServerConfig) {
c.Peering = nil // fix for older versions of Consul (<1.13.0) that don't support peering
c.Peering = nil // fix for older versions of Consul (<1.13.0) that don't support peering
// If -v wasn't specified squelch consul logging
if !testing.Verbose() {
c.Stdout = ioutil.Discard
Expand Down

0 comments on commit e2f9120

Please sign in to comment.