From fd7337c14153482532caff3ddf8a3a72a296e56c Mon Sep 17 00:00:00 2001
From: Luiz Aoqui <luiz@hashicorp.com>
Date: Mon, 26 Sep 2022 14:21:40 -0400
Subject: [PATCH] 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
---
 .changelog/14634.txt         |  3 +++
 command/operator_api.go      | 20 ++++++++++++---
 command/operator_api_test.go | 47 ++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 3 deletions(-)
 create mode 100644 .changelog/14634.txt

diff --git a/.changelog/14634.txt b/.changelog/14634.txt
new file mode 100644
index 000000000000..ccab596c8555
--- /dev/null
+++ b/.changelog/14634.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+cli: set content length on POST requests when using the `nomad operator api` command
+```
diff --git a/command/operator_api.go b/command/operator_api.go
index 9ff7b9c9a5fc..36dd375b241b 100644
--- a/command/operator_api.go
+++ b/command/operator_api.go
@@ -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"
 		}
diff --git a/command/operator_api_test.go b/command/operator_api_test.go
index 4292cd586cec..b71e65787f07 100644
--- a/command/operator_api_test.go
+++ b/command/operator_api_test.go
@@ -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")
+	}
+}