From 3587d8c2829e6fafdfad342e65a3f465a56080fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ramon=20R=C3=BCttimann?= <me@ramonr.ch>
Date: Mon, 17 Jul 2023 17:00:13 +0200
Subject: [PATCH] fix: escape and unescape name

This commit adds a small fix that starts path-(un)escaping the `name`
component of the purl.

This is required in case of the `name` containing a `/`, which would so
far have created a non-roundtrippable purl...
---
 packageurl.go      | 37 ++++++++++++++++++++++---------------
 packageurl_test.go | 14 ++++++++++++++
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/packageurl.go b/packageurl.go
index e1ef2ed..dc86d25 100644
--- a/packageurl.go
+++ b/packageurl.go
@@ -200,9 +200,14 @@ func (p *PackageURL) ToString() string {
 		Fragment: p.Subpath,
 	}
 
-	// we use JoinPath and EscapePath as the behavior for "/" is only correct with that.
+	nameWithVersion := url.PathEscape(p.Name)
+	if p.Version != "" {
+		nameWithVersion += "@" + p.Version
+	}
+
+	// we use JoinPath and EscapedPath as the behavior for "/" is only correct with that.
 	// We don't want to escape "/", but want to escape all other characters that are necessary.
-	u = u.JoinPath(p.Type, p.Namespace, strings.Join([]string{p.Name, p.Version}, "@"))
+	u = u.JoinPath(p.Type, p.Namespace, nameWithVersion)
 	// write the actual path into the "Opaque" block, so that the generated string at the end is
 	// pkg:<path> and not pkg://<path>.
 	u.Opaque, u.Path = u.EscapedPath(), ""
@@ -259,18 +264,27 @@ func FromString(purl string) (PackageURL, error) {
 }
 
 func separateNamespaceNameVersion(path string) (ns, name, version string, err error) {
-	namespaceSep := strings.LastIndex(path, "/")
-	if namespaceSep != -1 {
-		ns, err = url.PathUnescape(path[:namespaceSep])
+	name = path
+
+	if namespaceSep := strings.LastIndex(name, "/"); namespaceSep != -1 {
+		ns, name = name[:namespaceSep], name[namespaceSep+1:]
+
+		ns, err = url.PathUnescape(ns)
 		if err != nil {
 			return "", "", "", fmt.Errorf("error unescaping namespace: %w", err)
 		}
+	}
+
+	if versionSep := strings.LastIndex(name, "@"); versionSep != -1 {
+		name, version = name[:versionSep], name[versionSep+1:]
 
-		path = path[namespaceSep+1:]
+		version, err = url.PathUnescape(version)
+		if err != nil {
+			return "", "", "", fmt.Errorf("error unescaping version: %w", err)
+		}
 	}
 
-	v := strings.Split(path, "@")
-	name, err = url.PathUnescape(v[0])
+	name, err = url.PathUnescape(name)
 	if err != nil {
 		return "", "", "", fmt.Errorf("error unescaping name: %w", err)
 	}
@@ -279,13 +293,6 @@ func separateNamespaceNameVersion(path string) (ns, name, version string, err er
 		return "", "", "", fmt.Errorf("purl is missing name")
 	}
 
-	if len(v) > 1 {
-		version, err = url.PathUnescape(v[1])
-		if err != nil {
-			return "", "", "", fmt.Errorf("error unescaping version: %w", err)
-		}
-	}
-
 	return ns, name, version, nil
 }
 
diff --git a/packageurl_test.go b/packageurl_test.go
index ae1b325..2aac09c 100644
--- a/packageurl_test.go
+++ b/packageurl_test.go
@@ -297,7 +297,21 @@ func TestQualifiersMapConversion(t *testing.T) {
 			t.Logf("qualifiers -> map conversion failed: got: %#v, wanted: %#v", mp, test.kvMap)
 			t.Fail()
 		}
+	}
+}
 
+func TestNameEscaping(t *testing.T) {
+	testCases := map[string]string{
+		"abc":  "pkg:abc",
+		"ab/c": "pkg:ab%2Fc",
+	}
+	for name, output := range testCases {
+		t.Run(name, func(t *testing.T) {
+			p := &packageurl.PackageURL{Name: name}
+			if s := p.ToString(); s != output {
+				t.Fatalf("wrong escape. expected=%q, got=%q", output, s)
+			}
+		})
 	}
 
 }