cmd: make splitModule strictly conform to Go module spec (#7974)

Per https://go.dev/ref/mod#go-mod-file-ident, '@' is not a valid module
path character. The old implementation used strings.LastIndex to allow
inputs like github.com/@user/module@v1.0.0 to parse, but such inputs
are not valid module paths per the spec. Reject them explicitly.

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

cmd/packagesfuncs.go

@@ -49,20 +49,28 @@ func cmdUpgrade(fl Flags) (int, error) {
 func splitModule(arg string) (module, version string, err error) {
 	const versionSplit = "@"
 
-	// accommodate module paths that have @ in them, but we can only tolerate that if there's also
-	// a version, otherwise we don't know if it's a version separator or part of the file path
-	lastVersionSplit := strings.LastIndex(arg, versionSplit)
-	if lastVersionSplit < 0 {
+	// Per https://go.dev/ref/mod#go-mod-file-ident, module paths consist of
+	// ASCII letters, digits, and `-`, `.`, `_`, `~` only, so `@` is never a
+	// valid path character. That makes the split unambiguous: at most one
+	// `@` may appear, and it separates the module path from the version.
+	switch strings.Count(arg, versionSplit) {
+	case 0:
 		module = arg
-	} else {
-		module, version = arg[:lastVersionSplit], arg[lastVersionSplit+1:]
+	case 1:
+		idx := strings.Index(arg, versionSplit)
+		module, version = arg[:idx], arg[idx+1:]
+		if version == "" {
+			return "", "", fmt.Errorf("version is required after '@'")
+		}
+	default:
+		return "", "", fmt.Errorf("module path must not contain '@'")
 	}
 
 	if module == "" {
-		err = fmt.Errorf("module name is required")
+		return "", "", fmt.Errorf("module name is required")
 	}
 
-	return module, version, err
+	return module, version, nil
 }
 
 func cmdAddPackage(fl Flags) (int, error) {

cmd/packagesfuncs_split_test.go

@@ -0,0 +1,89 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package caddycmd
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestSplitModule_StrictGoSpec is the regression test for splitModule
+// silently accepting inputs that violate go.dev/ref/mod#go-mod-file-ident
+// (module paths cannot contain '@'). It also checks the happy paths and
+// the empty-version edge case.
+func TestSplitModule_StrictGoSpec(t *testing.T) {
+	for _, tt := range []struct {
+		name       string
+		input      string
+		module     string
+		version    string
+		wantErr    bool
+		wantErrSub string
+	}{
+		{
+			name:    "no version",
+			input:   "github.com/caddyserver/caddy",
+			module:  "github.com/caddyserver/caddy",
+			version: "",
+		},
+		{
+			name:    "with version",
+			input:   "github.com/caddyserver/caddy@v2.0.0",
+			module:  "github.com/caddyserver/caddy",
+			version: "v2.0.0",
+		},
+		{
+			name:       "multiple '@' rejected",
+			input:      "github.com/@user/module@v1.0.0",
+			wantErr:    true,
+			wantErrSub: "module path must not contain '@'",
+		},
+		{
+			name:       "trailing '@' with empty version rejected",
+			input:      "github.com/user/module@",
+			wantErr:    true,
+			wantErrSub: "version is required after '@'",
+		},
+		{
+			name:       "empty input rejected",
+			input:      "",
+			wantErr:    true,
+			wantErrSub: "module name is required",
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			module, version, err := splitModule(tt.input)
+
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("expected error containing %q, got nil", tt.wantErrSub)
+				}
+				if !strings.Contains(err.Error(), tt.wantErrSub) {
+					t.Errorf("error %q does not contain %q", err.Error(), tt.wantErrSub)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if module != tt.module {
+				t.Errorf("module: got %q, want %q", module, tt.module)
+			}
+			if version != tt.version {
+				t.Errorf("version: got %q, want %q", version, tt.version)
+			}
+		})
+	}
+}