[Backport 1.8] Add a new go-getter detector wrapper that can remove the query params before giving the src to the actual detector (#2459)

Signed-off-by: yottta <andrei.ciobanu@opentofu.org>
Signed-off-by: Christian Mesh <christianmesh1@gmail.com>
Co-authored-by: Christian Mesh <christianmesh1@gmail.com>

CHANGELOG.md

@@ -6,8 +6,10 @@ BUG FIXES:
 - `plantimestamp()` now returns unknown value during validation ([#2397](https://github.com/opentofu/opentofu/issues/2397))
 - Syntax error in the `required_providers` block does not panic anymore, but yields "syntax error" ([2344](https://github.com/opentofu/opentofu/issues/2344))
 - Fix the error message when default value of a complex variable is containing a wrong type ([2394](https://github.com/opentofu/opentofu/issues/2394))
+- Fix the way OpenTofu downloads a module that is sourced from a GitHub branch containing slashes in the name. ([2396](https://github.com/opentofu/opentofu/issues/2396))
 - Changing Go version to 1.22.11 in order to fix [CVE-2024-45336](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-45336) and [CVE-2024-45341](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-45341) ([#2438](https://github.com/opentofu/opentofu/pull/2438))
 
+
 ## 1.8.8
 
 SECURITY:

internal/addrs/module_source_test.go

@@ -138,6 +138,27 @@ func TestParseModuleSource(t *testing.T) {
 				Subdir:  "example/foo",
 			},
 		},
+		"github.com with branch and subdir": {
+			input: "github.com/hashicorp/terraform-cidr-subnets//example/foo?ref=bar",
+			want: ModuleSourceRemote{
+				Package: ModulePackage("git::https://github.com/hashicorp/terraform-cidr-subnets.git?ref=bar"),
+				Subdir:  "example/foo",
+			},
+		},
+		"github.com with subdir and malformed query params": {
+			input: "github.com/hashicorp/terraform-cidr-subnets//example/foo?",
+			want: ModuleSourceRemote{
+				Package: ModulePackage("git::https://github.com/hashicorp/terraform-cidr-subnets.git"),
+				Subdir:  "example/foo",
+			},
+		},
+		"github.com subdir from a branch containing slash in the name": {
+			input: "github.com/hashicorp/terraform-cidr-subnets//example/foo?ref=bar/baz",
+			want: ModuleSourceRemote{
+				Package: ModulePackage("git::https://github.com/hashicorp/terraform-cidr-subnets.git?ref=bar/baz"),
+				Subdir:  "example/foo",
+			},
+		},
 		"git protocol, URL-style": {
 			input: "git://example.com/code/baz.git",
 			want: ModuleSourceRemote{

internal/getmodules/getter.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"strings"
 
 	cleanhttp "github.com/hashicorp/go-cleanhttp"
 	getter "github.com/hashicorp/go-getter"
@@ -34,7 +35,7 @@ import (
 // tradeoffs we're making here.
 
 var goGetterDetectors = []getter.Detector{
-	new(getter.GitHubDetector),
+	&withoutQueryParams{d: new(getter.GitHubDetector)},
 	new(getter.GitDetector),
 
 	// Because historically BitBucket supported both Git and Mercurial
@@ -167,3 +168,25 @@ func (g reusingGetter) getWithGoGetter(ctx context.Context, instPath, packageAdd
 	// have got the full module package structure written into instPath.
 	return nil
 }
+
+// withoutQueryParams implements getter.Detector and can be used to wrap another detector.
+// This will look for any query params that might exist in the src and strip that away before calling
+// getter.Detector#Detect. After the response is returned, the query params are attached back to the resulted src.
+type withoutQueryParams struct {
+	d getter.Detector
+}
+
+func (w *withoutQueryParams) Detect(src string, pwd string) (string, bool, error) {
+	var qp string
+	if idx := strings.Index(src, "?"); idx > -1 {
+		qp = src[idx+1:]
+		src = src[:idx]
+	}
+
+	src, ok, err := w.d.Detect(src, pwd)
+	// Attach the query params only when the wrapped detector returns a value back
+	if len(src) > 0 && len(qp) > 0 {
+		src += "?" + qp
+	}
+	return src, ok, err
+}