Skip to content

Commit 90f8f55

Browse files
Refactor escaper usage to adhere to DRY, avoid reflection in tests using @VisibleForTesting, and improve validation clarity
1 parent e6023bc commit 90f8f55

2 files changed

Lines changed: 34 additions & 15 deletions

File tree

  • pulsar-package-management/core/src

pulsar-package-management/core/src/main/java/org/apache/pulsar/packages/management/core/common/PackageName.java

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.apache.pulsar.packages.management.core.common;
2020

21+
import com.google.common.annotations.VisibleForTesting;
2122
import com.google.common.base.Splitter;
2223
import com.google.common.base.Strings;
2324
import com.google.common.cache.CacheBuilder;
@@ -108,6 +109,18 @@ private PackageName(String packageName) {
108109
String.format("%s://%s/%s/%s@%s", type.toString(), tenant, namespace, name, version);
109110
}
110111

112+
@VisibleForTesting
113+
PackageName(PackageType type, String tenant, String namespace, String name, String version) {
114+
this.type = type;
115+
this.tenant = tenant;
116+
this.namespace = namespace;
117+
this.name = name;
118+
this.version = version;
119+
this.completeName = String.format("%s/%s/%s", tenant, namespace, name);
120+
this.completePackageName =
121+
String.format("%s://%s/%s/%s@%s", type.toString(), tenant, namespace, name, version);
122+
}
123+
111124
public PackageType getPkgType() {
112125
return this.type;
113126
}
@@ -137,13 +150,14 @@ public String toString() {
137150
}
138151

139152
public String toRestPath() {
140-
// Use Guava's urlPathSegmentEscaper to safely encode each segment and prevents Path Traversal (CWE-22)
153+
// Use Guava's URL path segment escaper to safely encode each segment and prevent path traversal (CWE-22).
154+
var escaper = UrlEscapers.urlPathSegmentEscaper();
141155
return String.format("%s/%s/%s/%s/%s",
142156
type.toString(),
143-
UrlEscapers.urlPathSegmentEscaper().escape(tenant),
144-
UrlEscapers.urlPathSegmentEscaper().escape(namespace),
145-
UrlEscapers.urlPathSegmentEscaper().escape(name),
146-
UrlEscapers.urlPathSegmentEscaper().escape(version));
157+
escaper.escape(tenant),
158+
escaper.escape(namespace),
159+
escaper.escape(name),
160+
escaper.escape(version));
147161
}
148162

149163
@Override

pulsar-package-management/core/src/test/java/org/apache/pulsar/packages/management/core/common/PackageNameTest.java

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -119,19 +119,24 @@ public void testPackageNameErrors() {
119119

120120
@Test
121121
public void testPathTraversalBypassConstructor() throws Exception {
122-
// Create a normal, valid package to bypass the splitter
123-
PackageName packageName = PackageName.get("function://tenant-a/ns/name@v1");
124-
125-
java.lang.reflect.Field tenantField = PackageName.class.getDeclaredField("tenant");
126-
tenantField.setAccessible(true);
127-
tenantField.set(packageName, "tenant-a/../../system-tenant");
128-
// Define what the SAFE, patched output should look like
122+
// Use the package-private constructor annotated with @VisibleForTesting
123+
// to inject a traversal payload directly, bypassing normal Splitter validation.
124+
PackageName packageName = new PackageName(
125+
PackageType.FUNCTION,
126+
"tenant-a/../../system-tenant",
127+
"ns",
128+
"name",
129+
"v1"
130+
);
131+
132+
// Verify that path separators in package components are percent-encoded in the generated REST path.
129133
String expectedSafePath = "function/tenant-a%2F..%2F..%2Fsystem-tenant/ns/name/v1";
130134

131-
// Trigger the vulnerable method
135+
// Invoke the method under test
132136
String actualPath = packageName.toRestPath();
133-
// The Trap: This will fail, because actualPath will still contain unencoded slashes
137+
138+
// This assertion verifies that traversal characters are encoded rather than emitted as raw slashes.
134139
Assert.assertEquals(actualPath, expectedSafePath,
135-
"VULNERABILITY REPLICATED: The toRestPath method failed to encode path traversal characters!");
140+
"The toRestPath method must encode path traversal characters in package components.");
136141
}
137142
}

0 commit comments

Comments
 (0)