weiqingy commented on code in PR #1005:
URL: https://github.com/apache/flink-agents/pull/1005#discussion_r3930991498
##########
api/src/test/java/org/apache/flink/agents/api/skills/SkillsResourceTest.java:
##########
@@ -48,6 +53,224 @@ void fromUrlEmitsUrlScheme() {
skills.getSources());
}
+ @Test
+ void fromUrlAcceptsSharedValidHostSyntax() {
+ for (String url :
+ List.of(
+ "https://localhost/x.zip";,
+ "https://127.0.0.1/x.zip";,
+ "https://[::1]/x.zip";,
+ "https://example.com./x.zip";,
+ "https://example.com:/x.zip";,
+ "https://example.com:65535/x.zip";,
+ "https://999/x.zip";,
+ "https://1bar/x.zip";,
+ "https://999./x.zip";)) {
+ assertEquals(url,
Skills.fromUrl(url).getSources().get(0).getParams().get("url"));
+ }
+ }
+
+ @Test
+ void fromUrlRejectsScopedIpv6() {
+ IllegalArgumentException ex =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> Skills.fromUrl("https://[fe80::1%25lo0]/x.zip";));
+ assertTrue(ex.getMessage().contains("must not include an IPv6 zone
identifier"));
+ assertNull(ex.getCause());
+ }
+
+ @Test
+ void fromUrlWithSha256EmitsIntegrityParam() {
+ String digest = "A".repeat(64);
+ Skills skills = Skills.fromUrlWithSha256("https://example.com/x.zip";,
digest);
+ assertEquals(
+ List.of(
+ new SkillSourceSpec(
+ "url",
+ Map.of("url", "https://example.com/x.zip";,
"sha256", digest))),
+ skills.getSources());
+ }
+
+ @Test
+ void fromUrlUnsafeRequiresExplicitParam() {
+ Skills skills = Skills.fromUrlUnsafe("http://example.com/x.zip";);
+ assertEquals("true",
skills.getSources().get(0).getParams().get("allow_insecure_http"));
+ }
+
+ @Test
+ void fromUrlUnsafeWithSha256EmitsBothParams() {
+ String digest = "a".repeat(64);
+ Skills skills =
Skills.fromUrlUnsafeWithSha256("http://example.com/x.zip";, digest);
+ assertEquals(
+ Map.of(
+ "url",
+ "http://example.com/x.zip";,
+ "sha256",
+ digest,
+ "allow_insecure_http",
+ "true"),
+ skills.getSources().get(0).getParams());
+ }
+
+ @Test
+ void fromUrlRejectsPlainHttpByDefault() {
+ assertThrows(
+ IllegalArgumentException.class, () ->
Skills.fromUrl("http://example.com/x.zip";));
+ }
+
+ @Test
+ void fromUrlWithSha256RejectsMalformedDigest() {
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> Skills.fromUrlWithSha256("https://example.com/x.zip";,
"invalid"));
+ }
+
+ @Test
+ void fromUrlRejectsUnsupportedSchemeClearly() {
+ IllegalArgumentException ex =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ Skills.fromUrl(
+
"ftp://user:[email protected]/x.zip?token=secret#part";));
+ assertEquals(
+ "Only HTTP(S) skill URLs are supported:
ftp://example.com/x.zip";, ex.getMessage());
+ assertFalse(ex.getMessage().contains("password"));
+ assertFalse(ex.getMessage().contains("secret"));
+ }
+
+ @Test
+ void fromUrlRejectsMalformedUrl() {
+ IllegalArgumentException unparseable =
+ assertThrows(
+ IllegalArgumentException.class, () ->
Skills.fromUrl("https://[::1/x.zip";));
+ assertEquals("Invalid skill URL: <redacted>",
unparseable.getMessage());
+
+ IllegalArgumentException unsafeForLogs =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
Skills.fromUrl("https://u:[email protected]/a\nheader?token=SECRET";));
+ assertEquals("Invalid skill URL: <redacted>",
unsafeForLogs.getMessage());
+ assertFalse(unsafeForLogs.getMessage().contains("pw"));
+ assertFalse(unsafeForLogs.getMessage().contains("SECRET"));
+
+ for (String url :
+ List.of("https://example.com/skills[1].zip";,
"https://[fe80::1%eth0]/x.zip";)) {
+ IllegalArgumentException ex =
+ assertThrows(IllegalArgumentException.class, () ->
Skills.fromUrl(url), url);
+ assertEquals("Invalid skill URL: " + url, ex.getMessage());
+ }
+ }
+
+ @Test
+ void fromUrlRedactsMalformedUrlWithoutLosingSafeContext() {
+ for (String path : List.of("a b.zip", "%zz")) {
+ IllegalArgumentException ex =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ Skills.fromUrl(
+ "https://u:[email protected]/";
+ + path
+ +
"?token=SECRET#fragment"));
+ assertEquals("Invalid skill URL: https://example.com/"; + path,
ex.getMessage());
+ assertFalse(ex.getMessage().contains("pw"));
+ assertFalse(ex.getMessage().contains("SECRET"));
+ }
+ }
+
+ @Test
+ void fromUrlAcceptsBracketsOutsideRawPath() {
+ for (String url :
+ List.of(
+ "https://example.com/skills%5B1%5D.zip";,
+ "https://example.com/x.zip?a[0]=1";,
"https://example.com/x.zip#f[1]";)) {
+ assertEquals(url,
Skills.fromUrl(url).getSources().get(0).getParams().get("url"));
+ }
+ }
+
+ @Test
+ void fromUrlRejectsInvalidHostAndPort() {
+ for (String url :
+ List.of(
+ "https://example.com:bad/x.zip?token=top-secret";,
+ "https://user:supersecret/x.zip?token=TOPSECRET";)) {
Review Comment:
Thanks for addressing the comments. Nothing further from me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
