From bfab7f6d54b762ec6e88b8daf836ac4f68fa488f Mon Sep 17 00:00:00 2001
From: Pablo Baeyens <pablo.baeyens@datadoghq.com>
Date: Thu, 25 Jul 2024 10:22:47 +0200
Subject: [PATCH] [confmap/provider/http(s)provider] Validate URIs before
 fetching (#10721)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
<!-- Issue number if applicable -->

Validate URLs before fetching.

#### Link to tracking issue
Fixes #10468, Relates to #10121
---
 .chloggen/mx-psi_validate-uris.yaml           | 25 ++++++++++++++++
 .../configurablehttpprovider/provider.go      |  5 ++++
 .../configurablehttpprovider/provider_test.go | 30 ++++++++++++++++---
 3 files changed, 56 insertions(+), 4 deletions(-)
 create mode 100644 .chloggen/mx-psi_validate-uris.yaml

diff --git a/.chloggen/mx-psi_validate-uris.yaml b/.chloggen/mx-psi_validate-uris.yaml
new file mode 100644
index 00000000000..69af525d77d
--- /dev/null
+++ b/.chloggen/mx-psi_validate-uris.yaml
@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: httpprovider, httpsprovider
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Validate URIs in HTTP and HTTPS providers before fetching.
+
+# One or more tracking issues or pull requests related to the change
+issues: [10468]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []
diff --git a/confmap/provider/internal/configurablehttpprovider/provider.go b/confmap/provider/internal/configurablehttpprovider/provider.go
index f5bac2c6d51..2d968e3bafe 100644
--- a/confmap/provider/internal/configurablehttpprovider/provider.go
+++ b/confmap/provider/internal/configurablehttpprovider/provider.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -84,6 +85,10 @@ func (fmp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFu
 		return nil, fmt.Errorf("%q uri is not supported by %q provider", uri, string(fmp.scheme))
 	}
 
+	if _, err := url.ParseRequestURI(uri); err != nil {
+		return nil, fmt.Errorf("invalid uri %q: %w", uri, err)
+	}
+
 	client, err := fmp.createClient()
 
 	if err != nil {
diff --git a/confmap/provider/internal/configurablehttpprovider/provider_test.go b/confmap/provider/internal/configurablehttpprovider/provider_test.go
index 125c1cbdd80..0561d51030a 100644
--- a/confmap/provider/internal/configurablehttpprovider/provider_test.go
+++ b/confmap/provider/internal/configurablehttpprovider/provider_test.go
@@ -297,9 +297,31 @@ func TestValidateProviderScheme(t *testing.T) {
 	assert.NoError(t, confmaptest.ValidateProviderScheme(New(HTTPScheme, confmaptest.NewNopProviderSettings())))
 }
 
-func TestInvalidTransport(t *testing.T) {
-	fp := New("foo", confmaptest.NewNopProviderSettings())
+func TestInvalidURI(t *testing.T) {
+	fp := New(HTTPScheme, confmaptest.NewNopProviderSettings())
 
-	_, err := fp.Retrieve(context.Background(), "foo://..", nil)
-	assert.Error(t, err)
+	tests := []struct {
+		uri string
+		err string
+	}{
+		{
+			uri: "foo://..",
+			err: "uri is not supported by \"http\" provider",
+		},
+		{
+			uri: "http://",
+			err: "no Host in request URL",
+		},
+		{
+			uri: "http://{}",
+			err: "invalid character \"{\" in host name",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.uri, func(t *testing.T) {
+			_, err := fp.Retrieve(context.Background(), tt.uri, nil)
+			assert.ErrorContains(t, err, tt.err)
+		})
+	}
 }