diff --git a/pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrl.java b/pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrl.java
index c6e76aa6..1929152c 100644
--- a/pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrl.java
+++ b/pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrl.java
@@ -31,9 +31,11 @@
* and host usually come from the request, and the script holds the URL in a
* single-quoted string, so the URL is checked before it is written into the
* script. A URL that is not valid, including one whose protocol is not http
- * or https, is refused. In the user information, in anything after the host
- * and port, and in the endpoint, the characters a URL cannot hold or that
- * could end the string or the script are percent-encoded.
+ * or https, is refused. In anything after the host and port, and in the
+ * endpoint, the characters a URL cannot hold or that could end the string or
+ * the script are percent-encoded. The rules are applied to the text given,
+ * and not to the result of parsing it, because a parser can accept hosts the
+ * rules refuse and rewrite others, such as 127.1.
* @see Specification
*/
final class CallbackUrl {
@@ -78,20 +80,21 @@ final class CallbackUrl {
private static final String[] SCHEMES = {"http", "https"};
/**
- * A label of a host name, being ASCII letters, digits, '-' and '_'.
+ * The characters the host value cannot hold. A '?' or '#' would make the
+ * endpoint part of the query or fragment, and an '@' means user
+ * information, which fetch refuses and XMLHttpRequest sends.
*/
- private static final Pattern LABEL = Pattern.compile("[A-Za-z0-9_-]+");
+ private static final String NOT_IN_HOST = "?#@";
/**
- * An IPv6 zone identifier, being unreserved characters as RFC 3986
- * section 2.3 defines them.
+ * A label of a host name, being ASCII letters, digits, '-' and '_'.
*/
- private static final Pattern ZONE = Pattern.compile("[A-Za-z0-9._~-]+");
+ private static final Pattern LABEL = Pattern.compile("[A-Za-z0-9_-]+");
/**
- * A port as it follows the host, being ':' and any number of digits.
+ * A port as it follows the host, being ':' and at least one digit.
*/
- private static final Pattern PORT = Pattern.compile(":[0-9]*");
+ private static final Pattern PORT = Pattern.compile(":[0-9]+");
/**
* One group of an IPv6 address.
@@ -107,6 +110,13 @@ final class CallbackUrl {
"(25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9]?[0-9])" +
"(\\.(25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9]?[0-9])){3}");
+ /**
+ * A label that the URL Standard reads as a number, being decimal digits
+ * or "0x" followed by hexadecimal digits.
+ */
+ private static final Pattern NUMBER =
+ Pattern.compile("[0-9]+|0[xX][0-9A-Fa-f]*");
+
/**
* Whether the URL can be written into the script.
*/
@@ -114,7 +124,8 @@ final class CallbackUrl {
/**
* The URL to write into the script when it is valid, and otherwise the
- * URL as it was built, for the log.
+ * URL as it was built, for the log, with *** in place of everything in
+ * the host value before its last '@'.
*/
final String url;
@@ -129,8 +140,7 @@ private CallbackUrl(boolean valid, String url) {
* and removed when both do. The protocol and the host are written in
* lower case and the rest as given, apart from the percent-encoding.
* @param protocol the protocol, http or https in any case
- * @param host the host, which can start with user information and end
- * with a port, a path, a query or a fragment
+ * @param host the host, which can end with a port and a path
* @param endpoint the endpoint
* @return the callback URL, or null when the protocol, host or endpoint
* is not set
@@ -146,14 +156,14 @@ static CallbackUrl of(String protocol, String host, String endpoint) {
} else if (hostSlash && endpointSlash) {
endpoint = endpoint.substring(1);
}
- String built = protocol + "://" + host + endpoint;
- if (isScheme(protocol) == false) {
- return new CallbackUrl(false, built);
- }
int end = authorityEnd(host);
- String authority = checkedAuthority(host.substring(0, end));
+ String authority = isScheme(protocol) && hasNoForbiddenCharacter(host)
+ ? checkedHostPort(host.substring(0, end))
+ : null;
if (authority == null) {
- return new CallbackUrl(false, built);
+ return new CallbackUrl(
+ false,
+ protocol + "://" + withoutUserInformation(host) + endpoint);
}
return new CallbackUrl(
true,
@@ -161,6 +171,28 @@ static CallbackUrl of(String protocol, String host, String endpoint) {
encoded(host.substring(end)) + encoded(endpoint));
}
+ /**
+ * Whether the host value holds none of '?', '#' and '@'.
+ */
+ private static boolean hasNoForbiddenCharacter(String host) {
+ for (int i = 0; i < host.length(); i++) {
+ if (NOT_IN_HOST.indexOf(host.charAt(i)) >= 0) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * The host value with everything before its last '@' replaced by ***.
+ * The last '@' of the whole value is used, and not that of the
+ * authority, because a password can hold a '/', '?' or '#'.
+ */
+ private static String withoutUserInformation(String host) {
+ int at = host.lastIndexOf('@');
+ return at < 0 ? host : "***" + host.substring(at);
+ }
+
/**
* Whether the protocol is http or https, ignoring the case of its ASCII
* letters only. String.equalsIgnoreCase also matches a letter outside
@@ -208,41 +240,20 @@ static boolean isBlank(String text) {
}
/**
- * Where the authority ends, which is at the first '/', '?' or '#' of the
- * host, or at its end.
+ * Where the authority ends, which is at the first '/' of the host, or at
+ * its end.
*/
private static int authorityEnd(String host) {
- for (int i = 0; i < host.length(); i++) {
- char c = host.charAt(i);
- if (c == '/' || c == '?' || c == '#') {
- return i;
- }
- }
- return host.length();
- }
-
- /**
- * The user information, host and port as they are written into the URL,
- * or null when they are not valid. The user information ends at the
- * first '@', so a second '@' is part of the host and refused.
- */
- private static String checkedAuthority(String authority) {
- int at = authority.indexOf('@');
- if (at < 0) {
- return checkedHostPort(authority);
- }
- String hostPort = checkedHostPort(authority.substring(at + 1));
- return hostPort == null
- ? null
- : encoded(authority.substring(0, at)) + "@" + hostPort;
+ int end = host.indexOf('/');
+ return end < 0 ? host.length() : end;
}
/**
* The host and optional port in lower case, or null when they are not
- * valid. The host is an IPv6 address in brackets with an optional zone
- * identifier, or a name of labels separated by dots with a dot allowed
- * at the end. The port is ':' followed by digits up to 65535, or by
- * nothing.
+ * valid. The host is an IPv6 address in brackets, or a name of labels
+ * separated by dots with a dot allowed at the end, which must be an IPv4
+ * address when its last label is a number. The port is ':' followed by
+ * digits up to 65535.
*/
private static String checkedHostPort(String hostPort) {
String host;
@@ -253,14 +264,6 @@ private static String checkedHostPort(String hostPort) {
return null;
}
String address = hostPort.substring(1, close);
- int zone = address.indexOf("%25");
- if (zone >= 0) {
- if (ZONE.matcher(address.substring(zone + 3)).matches() ==
- false) {
- return null;
- }
- address = address.substring(0, zone);
- }
if (isIpv6(address) == false) {
return null;
}
@@ -274,11 +277,16 @@ private static String checkedHostPort(String hostPort) {
String name = host.endsWith(".")
? host.substring(0, host.length() - 1)
: host;
- for (String label : name.split("\\.", -1)) {
+ String[] labels = name.split("\\.", -1);
+ for (String label : labels) {
if (LABEL.matcher(label).matches() == false) {
return null;
}
}
+ if (NUMBER.matcher(labels[labels.length - 1]).matches() &&
+ IPV4.matcher(host).matches() == false) {
+ return null;
+ }
}
if (port.isEmpty() == false && isPort(port) == false) {
return null;
@@ -288,7 +296,7 @@ private static String checkedHostPort(String hostPort) {
/**
* Whether the text is ':' followed by digits whose value is no more than
- * 65535, or by nothing.
+ * 65535.
*/
private static boolean isPort(String text) {
if (PORT.matcher(text).matches() == false) {
diff --git a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptUrlTests.java b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptUrlTests.java
index 67006a69..fc8cb2f6 100644
--- a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptUrlTests.java
+++ b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptUrlTests.java
@@ -34,6 +34,7 @@
import fiftyone.pipeline.jsonbuilder.flowelements.JsonBuilderElementBuilder;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.slf4j.ILoggerFactory;
import org.slf4j.Logger;
@@ -230,6 +231,34 @@ public void Url_RefusedHost_NoCallbackAndLoggedOnce() throws Exception {
errors.get(0));
}
+ /**
+ * A refused host is logged with *** in place of everything in the host
+ * value before its last '@', so a password is never written to the log.
+ * Each case has its own element, because a refusal is logged once per
+ * element.
+ */
+ @ParameterizedTest(name = "[{index}]")
+ @CsvSource(delimiter = '|', value = {
+ "user:secret@example.com|\"https://***@example.com/json\"",
+ "a@b@example.com|\"https://***@example.com/json\"",
+ "u@[::1]:80x|\"https://***@[::1]:80x/json\"",
+ "user:pa/ss@example.com|\"https://***@example.com/json\"",
+ "user:pa?ss@example.com|\"https://***@example.com/json\"",
+ "exa mple.com/a@b|\"https://***@b/json\"",
+ "example.123|\"https://example.123/json\"",
+ "https://example.com|\"https://https://example.com/json\""})
+ public void Url_RefusedHost_LoggedWithoutUserInformation(
+ String host, String logged) throws Exception {
+ renderWithHosts(host);
+
+ List errors = builderErrors();
+ assertEquals(1, errors.size(), String.join("\n", errors));
+ assertTrue(errors.get(0).contains(logged), errors.get(0));
+ for (String secret : new String[] {"secret", "pa/", "pa?", "ss@"}) {
+ assertFalse(errors.get(0).contains(secret), errors.get(0));
+ }
+ }
+
/**
* A protocol from the request that is neither http nor https gives a
* script with no callback, and is logged once like any other URL that
@@ -273,7 +302,8 @@ public void Url_RefusedProtocol_NoCallbackAndLoggedOnce() throws Exception {
* refused too, which String.equalsIgnoreCase would match to 's'.
*/
@ParameterizedTest
- @ValueSource(strings = {"ftp", "ws", "javascript", "https ", "http\u017f"})
+ @ValueSource(strings = {"ftp", "ws", "javascript", "https ", "http\u017f",
+ " ", "\t", "\u00a0"})
public void Protocol_ConfiguredOtherThanHttpOrHttps_IsRefused(
String protocol) {
JavaScriptBuilderElementBuilder builder =
diff --git a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrlTests.java b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrlTests.java
index b4be236e..12b7e3c9 100644
--- a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrlTests.java
+++ b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrlTests.java
@@ -99,8 +99,6 @@ static Stream acceptedHosts() {
"https://example.com/51dpipeline/json"),
Arguments.of("example.com:8080",
"https://example.com:8080/51dpipeline/json"),
- Arguments.of("example.com:",
- "https://example.com:/51dpipeline/json"),
Arguments.of("example.com:65535",
"https://example.com:65535/51dpipeline/json"),
Arguments.of("a_b.com",
@@ -109,26 +107,38 @@ static Stream acceptedHosts() {
"https://-ab.com/51dpipeline/json"),
Arguments.of("127.0.0.1:80",
"https://127.0.0.1:80/51dpipeline/json"),
+ Arguments.of("example.com",
+ "https://example.com/51dpipeline/json"),
+ Arguments.of("1.2.3.4",
+ "https://1.2.3.4/51dpipeline/json"),
+ Arguments.of("1.2.3.4:8080",
+ "https://1.2.3.4:8080/51dpipeline/json"),
+ Arguments.of("1.2.3.4/path",
+ "https://1.2.3.4/path/51dpipeline/json"),
+ Arguments.of("0.0.0.0",
+ "https://0.0.0.0/51dpipeline/json"),
+ Arguments.of("101.2.3.4",
+ "https://101.2.3.4/51dpipeline/json"),
+ Arguments.of("example.co1",
+ "https://example.co1/51dpipeline/json"),
+ Arguments.of("123.example.com",
+ "https://123.example.com/51dpipeline/json"),
Arguments.of("[::1]",
"https://[::1]/51dpipeline/json"),
Arguments.of("[::1]:8080",
"https://[::1]:8080/51dpipeline/json"),
- Arguments.of("[fe80::1%25eth0]",
- "https://[fe80::1%25eth0]/51dpipeline/json"),
- Arguments.of("user:pw@example.com",
- "https://user:pw@example.com/51dpipeline/json"),
- Arguments.of("u x@example.com",
- "https://u%20x@example.com/51dpipeline/json"),
- Arguments.of("u\tx@example.com",
- "https://u%09x@example.com/51dpipeline/json"),
+ Arguments.of("[1:2:3:4:5:6:7:8]",
+ "https://[1:2:3:4:5:6:7:8]/51dpipeline/json"),
+ Arguments.of("[::1.2.3.4]",
+ "https://[::1.2.3.4]/51dpipeline/json"),
Arguments.of("example.com.",
"https://example.com./51dpipeline/json"),
Arguments.of("example.com/path",
"https://example.com/path/51dpipeline/json"),
- Arguments.of("example.com?x",
- "https://example.com?x/51dpipeline/json"),
Arguments.of("example.com/a b",
"https://example.com/a%20b/51dpipeline/json"),
+ Arguments.of("example.com/a\"b",
+ "https://example.com/a%22b/51dpipeline/json"),
Arguments.of("example.com/a%zzb",
"https://example.com/a%25zzb/51dpipeline/json"),
Arguments.of("example.com/a%41b",
@@ -183,7 +193,63 @@ static Stream refusedHosts() {
"a\nb.com",
" example.com",
"example.com ",
- "example.com:8080');alert(1);//");
+ "example.com\n",
+ "example.com:80\n",
+ "[::1]\n",
+ "example.com:8080');alert(1);//",
+ // User information, or an @ anywhere in the host value.
+ "user@example.com",
+ "user:pw@example.com",
+ "@example.com",
+ ":@example.com",
+ "user@example.com:8080",
+ "user@[::1]",
+ "user@example.com/path",
+ "u%41@example.com",
+ "u x@example.com",
+ "u\tx@example.com",
+ "u\"x@example.com",
+ "u'x@example.com",
+ "u\\x@example.com",
+ "u\nx@example.com",
+ "u\u0001x@example.com",
+ "example.com/a@b",
+ "user:12/ss@example.com",
+ "user:pa/ss@example.com",
+ "user:pa?ss@example.com",
+ "exa mple.com/a@b",
+ // A query or fragment.
+ "example.com?x",
+ "example.com#x",
+ "example.com?",
+ "example.com#",
+ "example.com:8080?x",
+ // A port that is present but empty.
+ "example.com:",
+ "example.com:/path",
+ "[::1]:",
+ "https://example.com",
+ "http://example.com",
+ "[::1]#x",
+ "example.com/path?x",
+ "example.com/path#x",
+ "example.com?');alert(1);//",
+ "example.com#');alert(1);//",
+ // A name whose last label is a number must be an IPv4 address.
+ "example.123",
+ "example.0x1f",
+ "example.0X",
+ "1.2.3.999",
+ "1.2.3",
+ "127.1",
+ "0x7f.0.0.1",
+ "01.2.3.4",
+ "example.123.",
+ "1.2.3.4.",
+ "1.2.3.4.5",
+ "1",
+ "300.1.1.1",
+ "0x7f.1");
}
@ParameterizedTest(name = "[{index}]")
@@ -195,20 +261,27 @@ public void CallbackUrl_RefusesHost(String host) {
static Stream refusedZonesAndTextAfterTheBracket() {
return Stream.of(
"[fe80::1%25]",
+ "[fe80::1%25eth0]",
+ "[fe80::1%25eth0]:8080",
+ "[FE80::1%25ETH0]",
+ "[fe80::1%251]",
+ "[::1%25lo]",
+ "[fe80::1%]",
+ "[fe80::1%25eth0]/path",
"[fe80::1%25'+alert(1)+']",
"[fe80::1%eth0]",
"[fe80::1%25eth%0]",
"[::1]x",
"[::1]80",
"[::1]:80x",
- "[::1]]");
+ "[::1]]",
+ "[1::2::3]",
+ "[12345::1]");
}
/**
- * Inside the brackets only an IPv6 address, optionally followed by '%25'
- * and a zone of unreserved characters, and after them only a port. The
- * zone check is what keeps a quote out of the script for a bracketed
- * host.
+ * Inside the brackets only an IPv6 address, so a zone identifier is
+ * refused, and after them only a port.
*/
@ParameterizedTest(name = "[{index}]")
@MethodSource("refusedZonesAndTextAfterTheBracket")
@@ -216,14 +289,6 @@ public void CallbackUrl_RefusesZoneOrTextAfterTheBracket(String host) {
assertInvalid(url("https", host), "host " + host);
}
- @Test
- public void CallbackUrl_AcceptsZoneFollowedByPort() {
- assertValid(
- "https://[fe80::1%25eth0]:8080/51dpipeline/json",
- url("https", "[fe80::1%25eth0]:8080"),
- "host [fe80::1%25eth0]:8080");
- }
-
/**
* A port goes by its value, so leading zeros are allowed, and a port too
* long to read is refused rather than read as small enough.
@@ -248,17 +313,14 @@ public void CallbackUrl_ChecksThePortByItsValue() {
}
/**
- * Each character rule 8 lists, in the user information, after the host
- * and in the endpoint, and a '%' without two hexadecimal digits after
- * it, even at the end.
+ * Each character rule 8 lists, after the host and in the endpoint, and a
+ * '%' without two hexadecimal digits after it, even at the end.
*/
@Test
public void CallbackUrl_EncodesTheCharactersRule8Lists() {
assertValid(
- "https://u%22%5E%60%7B%7C%7D@example.com/%22%5E%60%7B%7C%7D" +
- "/j%22%5E%60%7B%7C%7D",
- CallbackUrl.of(
- "https", "u\"^`{|}@example.com/\"^`{|}", "/j\"^`{|}"),
+ "https://example.com/%22%5E%60%7B%7C%7D/j%22%5E%60%7B%7C%7D",
+ CallbackUrl.of("https", "example.com/\"^`{|}", "/j\"^`{|}"),
"rule 8 characters");
assertValid(
"https://example.com/a%25/51dpipeline/json",
@@ -278,6 +340,37 @@ public void CallbackUrl_EncodesTheCharactersRule8Lists() {
"endpoint /json%");
}
+ static Stream refusedUrlsForTheLog() {
+ return Stream.of(
+ Arguments.of("user:secret@example.com",
+ "https://***@example.com/51dpipeline/json"),
+ Arguments.of("a@b@example.com",
+ "https://***@example.com/51dpipeline/json"),
+ Arguments.of("u@[::1]:80x",
+ "https://***@[::1]:80x/51dpipeline/json"),
+ Arguments.of("user:pa/ss@example.com",
+ "https://***@example.com/51dpipeline/json"),
+ Arguments.of("user:pa?ss@example.com",
+ "https://***@example.com/51dpipeline/json"),
+ Arguments.of("exa mple.com/a@b",
+ "https://***@b/51dpipeline/json"),
+ Arguments.of("@example.com",
+ "https://***@example.com/51dpipeline/json"));
+ }
+
+ /**
+ * The URL kept for the log shows *** in place of everything in the host
+ * value before its last '@'.
+ */
+ @ParameterizedTest(name = "[{index}]")
+ @MethodSource("refusedUrlsForTheLog")
+ public void CallbackUrl_RefusedUrlForTheLogHasNoUserInformation(
+ String host, String expected) {
+ CallbackUrl url = url("https", host);
+ assertInvalid(url, "host " + host);
+ assertEquals(expected, url.url, "host " + host);
+ }
+
@Test
public void CallbackUrl_InvalidUrlIsTheOneBuilt() {
CallbackUrl url = url("https", "exa mple.com");
@@ -317,10 +410,7 @@ public void CallbackUrl_RefusesScheme(String protocol) {
static Stream hostsThatCouldEndTheString() {
return Stream.of(
"example.com/');alert(1);//",
- "example.com?');alert(1);//",
- "example.com#');alert(1);//",
- "u'x@example.com",
- "u\\x@example.com",
+ "example.com/a\"b",
"example.com/a\\b",
"example.com/");
}