cshannon commented on code in PR #1950:
URL: https://github.com/apache/activemq/pull/1950#discussion_r3117690524
##########
activemq-spring/src/main/java/org/apache/activemq/spring/Utils.java:
##########
@@ -77,22 +80,51 @@ public static Resource resourceFromString(String uri,
Set<String> allowedProtoco
return resource;
}
- static boolean isAllowFile(Set<String> allowedProtocols) {
- return allowedProtocols == null ||
allowedProtocols.contains(FILE_PROTOCOL);
+ // These method treats local files and remote files (that are pre-fixed
+ // with two forward/backward slashes) differently
+ static boolean isAllowFile(Set<String> allowedProtocols, String uri) {
+ if (allowedProtocols == null) {
+ return true;
+ }
+ return isUnqualifiedRemoteFile(uri) ?
allowedProtocols.contains(REMOTE_FILE_PROTOCOL) :
+ allowedProtocols.contains(FILE_PROTOCOL);
}
static boolean isAllowClasspath(Set<String> allowedProtocols) {
return allowedProtocols == null ||
allowedProtocols.contains(CLASSPATH_PROTOCOL);
}
- private static void validateUrlAllowed(String uriString, Set<String>
allowedProtocols)
+ static void validateUrlAllowed(String uriString, Set<String>
allowedProtocols)
throws URISyntaxException {
// Use new URI() to get the scheme
// This is important because ResourceUtils.getURL() actually searches
// the classpath which we don't want to do if not allowed
- if (allowedProtocols != null && !allowedProtocols.contains(new
URI(uriString).getScheme())) {
- throw new IllegalArgumentException("URL [" + uriString +
- "] does not use an allowed protocol for loading URL
resources");
+ if (allowedProtocols != null) {
+ final String detectedProtocol = getProtocolFromScheme(uriString);
+ if (detectedProtocol == null) {
+ throw new IllegalArgumentException("Could not detect protocol
in given URI [" + uriString + "]");
+ }
+ if (!allowedProtocols.contains(detectedProtocol)){
+ throw new IllegalArgumentException("URL [" + uriString +
+ "] uses protocol '" + detectedProtocol + "' which is
not allowed "
+ + "for loading URL resources");
+ }
}
}
+
+ // If this is a qualified remote file then we return a special marker
+ // so that we know this is trying to access a remote resource as that will
be
+ // validated differently
+ private static String getProtocolFromScheme(String uriString) throws
URISyntaxException {
+ return isQualifiedRemoteFile(uriString) ? REMOTE_FILE_PROTOCOL :
+ new URI(uriString).getScheme();
+ }
+
+ private static boolean isUnqualifiedRemoteFile(String uri) {
+ return uri.startsWith("//") || uri.startsWith("\\\\");
+ }
+
+ private static boolean isQualifiedRemoteFile(String uri) {
+ return uri.startsWith(FILE_PROTOCOL + "://") ||
uri.startsWith(FILE_PROTOCOL + ":\\\\");
Review Comment:
That is a good point but I think the safest thing here is to just only allow
using the single slash or relative path. No one needs to use `file:///` they
can just use `file:/` and that will simplify things and prevent any bugs.
We can make a note of that when this is released and we write some
documentation.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
