cli: set content length on `operator api` requests (#14634)
http.NewRequestWithContext will only set the right value for Content-Length if the input is *bytes.Buffer, *bytes.Reader, or *strings.Reader [0]. Since os.Stdin is an os.File, POST requests made with the `nomad operator api` command would always have Content-Length set to -1, which is interpreted as an unknown length by web servers. [0]: https://pkg.go.dev/net/http#NewRequestWithContext
This commit is contained in:
parent
cdb3ec25d3
commit
f7c6534a79
|
@ -0,0 +1,3 @@
|
|||
```release-note:bug
|
||||
cli: set content length on POST requests when using the `nomad operator api` command
|
||||
```
|
|
@ -1,9 +1,11 @@
|
|||
package command
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"crypto/tls"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/ioutil"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
|
@ -16,6 +18,10 @@ import (
|
|||
"github.com/posener/complete"
|
||||
)
|
||||
|
||||
// Stdin represents the system's standard input, but it's declared as a
|
||||
// variable here to allow tests to override it with a regular file.
|
||||
var Stdin = os.Stdin
|
||||
|
||||
type OperatorAPICommand struct {
|
||||
Meta
|
||||
|
||||
|
@ -31,7 +37,7 @@ Usage: nomad operator api [options] <path>
|
|||
api is a utility command for accessing Nomad's HTTP API and is inspired by
|
||||
the popular curl command line tool. Nomad's operator api command populates
|
||||
Nomad's standard environment variables into their appropriate HTTP headers.
|
||||
If the 'path' does not begin with "http" then $NOMAD_ADDR will be used.
|
||||
If the 'path' does not begin with "http" then $NOMAD_ADDR will be used.
|
||||
|
||||
The 'path' can be in one of the following forms:
|
||||
|
||||
|
@ -139,10 +145,18 @@ func (c *OperatorAPICommand) Run(args []string) int {
|
|||
|
||||
// Opportunistically read from stdin and POST unless method has been
|
||||
// explicitly set.
|
||||
stat, _ := os.Stdin.Stat()
|
||||
stat, _ := Stdin.Stat()
|
||||
if (stat.Mode() & os.ModeCharDevice) == 0 {
|
||||
verbose("* Reading request body from stdin.")
|
||||
c.body = os.Stdin
|
||||
|
||||
// Load stdin into a *bytes.Reader so that http.NewRequest can set the
|
||||
// correct Content-Length value.
|
||||
b, err := ioutil.ReadAll(Stdin)
|
||||
if err != nil {
|
||||
c.Ui.Error(fmt.Sprintf("Error reading stdin: %v", err))
|
||||
return 1
|
||||
}
|
||||
c.body = bytes.NewReader(b)
|
||||
if c.method == "" {
|
||||
c.method = "POST"
|
||||
}
|
||||
|
|
|
@ -5,6 +5,7 @@ import (
|
|||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"os"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
|
@ -170,3 +171,49 @@ func Test_pathToURL(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestOperatorAPICommand_ContentLength tests that requests have the proper
|
||||
// ContentLength set.
|
||||
//
|
||||
// Don't run it in parallel as it modifies the package's Stdin variable.
|
||||
func TestOperatorAPICommand_ContentLength(t *testing.T) {
|
||||
contentLength := make(chan int, 1)
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
contentLength <- int(r.ContentLength)
|
||||
}))
|
||||
defer ts.Close()
|
||||
|
||||
// Setup a temp file to act as stdin.
|
||||
input := []byte("test-input")
|
||||
fakeStdin, err := os.CreateTemp("", "fake-stdin")
|
||||
require.NoError(t, err)
|
||||
defer os.Remove(fakeStdin.Name())
|
||||
|
||||
_, err = fakeStdin.Write(input)
|
||||
require.NoError(t, err)
|
||||
_, err = fakeStdin.Seek(0, 0)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Override the package's Stdin variable for testing.
|
||||
Stdin = fakeStdin
|
||||
defer func() { Stdin = os.Stdin }()
|
||||
|
||||
// Setup command.
|
||||
buf := bytes.NewBuffer(nil)
|
||||
ui := &cli.BasicUi{
|
||||
ErrorWriter: buf,
|
||||
Writer: buf,
|
||||
}
|
||||
cmd := &OperatorAPICommand{Meta: Meta{Ui: ui}}
|
||||
|
||||
// Assert that a request has the expected content length.
|
||||
exitCode := cmd.Run([]string{"-address=" + ts.URL, "/v1/jobs"})
|
||||
require.Zero(t, exitCode, buf.String())
|
||||
|
||||
select {
|
||||
case l := <-contentLength:
|
||||
require.Equal(t, len(input), l)
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatalf("timed out waiting for request")
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue