Repository: hetznercloud/hcloud-go (go)

Issue:
fix(metadata): trim responses before parsing
### TL;DR

Responses from metadata service are not trimmed before parsing. They rely on underspecified behaviour of the metadata service api.

### Expected behavior

Before parsing the response, we should at least trim any excessive whitespace

### Observed behavior

If the response contains any whitespace (like a newline at the end of it), the parsing of some of the fields fails.

This applies to `InstanceID()`, `PublicIPv4()`.

In addition, the return values of all other functions would also include the additional whitespace, leading to unexpected results downstream.

### Minimal working example

_No response_

### Log output

_No response_

### Additional information

_No response_

Pull request:
fix(metadata): trim responses before parsing
Fixes #540

Patch:
diff --git a/hcloud/metadata/client.go b/hcloud/metadata/client.go
index f9634c3..b873cb2 100644
--- a/hcloud/metadata/client.go
+++ b/hcloud/metadata/client.go
@@ -1,6 +1,7 @@
 package metadata
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"net"
@@ -80,16 +81,20 @@ func NewClient(options ...ClientOption) *Client {
 // get executes an HTTP request against the API.
 func (c *Client) get(path string) (string, error) {
 	url := c.endpoint + path
+
 	resp, err := c.httpClient.Get(url)
 	if err != nil {
 		return "", err
 	}
+
 	defer resp.Body.Close()
 	bodyBytes, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return "", err
 	}
-	body := string(bodyBytes)
+
+	body := string(bytes.TrimSpace(bodyBytes))
+
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		return body, fmt.Errorf("response status was %d", resp.StatusCode)
 	}


Test patch:
diff --git a/hcloud/metadata/client_test.go b/hcloud/metadata/client_test.go
index ba1e59e..ab50499 100644
--- a/hcloud/metadata/client_test.go
+++ b/hcloud/metadata/client_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type testEnv struct {
@@ -39,6 +40,9 @@ func TestClient_Base(t *testing.T) {
 	env.Mux.HandleFunc("/ok", func(w http.ResponseWriter, r *http.Request) {
 		w.Write([]byte("ok"))
 	})
+	env.Mux.HandleFunc("/sanitized", func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte("sanitized  \n"))
+	})
 	env.Mux.HandleFunc("/not-found", func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(404)
 		w.Write([]byte("not found"))
@@ -47,6 +51,9 @@ func TestClient_Base(t *testing.T) {
 	if body, err := env.Client.get("/ok"); assert.NoError(t, err) {
 		assert.Equal(t, "ok", body)
 	}
+	if body, err := env.Client.get("/sanitized"); assert.NoError(t, err) {
+		assert.Equal(t, "sanitized", body)
+	}
 	if body, err := env.Client.get("/not-found"); assert.EqualError(t, err, "response status was 404") {
 		assert.Equal(t, "not found", body)
 	}
@@ -99,33 +106,51 @@ func TestClient_Hostname(t *testing.T) {
 }
 
 func TestClient_InstanceID(t *testing.T) {
-	env := newTestEnv()
-	env.Mux.HandleFunc("/instance-id", func(w http.ResponseWriter, r *http.Request) {
-		w.Write([]byte("123456"))
+	t.Run("success", func(t *testing.T) {
+		env := newTestEnv()
+		env.Mux.HandleFunc("/instance-id", func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte("123456"))
+		})
+
+		instanceID, err := env.Client.InstanceID()
+		require.NoError(t, err)
+		require.Equal(t, int64(123456), instanceID)
 	})
 
-	instanceID, err := env.Client.InstanceID()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if instanceID != 123456 {
-		t.Fatalf("Unexpected instanceID %d", instanceID)
-	}
+	t.Run("success sanitized", func(t *testing.T) {
+		env := newTestEnv()
+		env.Mux.HandleFunc("/instance-id", func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte("123456\n"))
+		})
+
+		instanceID, err := env.Client.InstanceID()
+		require.NoError(t, err)
+		require.Equal(t, int64(123456), instanceID)
+	})
 }
 
 func TestClient_PublicIPv4(t *testing.T) {
-	env := newTestEnv()
-	env.Mux.HandleFunc("/public-ipv4", func(w http.ResponseWriter, r *http.Request) {
-		w.Write([]byte("127.0.0.1"))
+	t.Run("success", func(t *testing.T) {
+		env := newTestEnv()
+		env.Mux.HandleFunc("/public-ipv4", func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte("127.0.0.1"))
+		})
+
+		publicIPv4, err := env.Client.PublicIPv4()
+		require.NoError(t, err)
+		require.Equal(t, "127.0.0.1", publicIPv4.String())
 	})
 
-	publicIPv4, err := env.Client.PublicIPv4()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if publicIPv4.String() != "127.0.0.1" {
-		t.Fatalf("Unexpected PublicIPv4 %s", publicIPv4.String())
-	}
+	t.Run("success sanitized", func(t *testing.T) {
+		env := newTestEnv()
+		env.Mux.HandleFunc("/public-ipv4", func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte("127.0.0.1\n"))
+		})
+
+		publicIPv4, err := env.Client.PublicIPv4()
+		require.NoError(t, err)
+		require.Equal(t, "127.0.0.1", publicIPv4.String())
+	})
 }
 
 func TestClient_Region(t *testing.T) {