Handle relative URIs with ":" in path names correctly

app/build.gradle

@@ -12,9 +12,6 @@ android {
 
     buildTypes {
         debug {
-            //minifyEnabled false
-            minifyEnabled true
-            proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.txt'
         }
         release {
             minifyEnabled true

app/proguard-rules.txt

@@ -32,4 +32,3 @@
 
 # DAVdroid
 -keep class at.bitfire.davdroid.** { *; }	# all DAVdroid code is required
-

app/src/androidTest/java/at/bitfire/davdroid/URLUtilsTest.java

@@ -35,7 +35,7 @@ public class URLUtilsTest extends TestCase {
 		assertEquals("/test/", URIUtils.ensureTrailingSlash("/test"));
 		assertEquals("/test/", URIUtils.ensureTrailingSlash("/test/"));
 
-		String	withoutSlash = "http://www.test.at/dav/collection",
+		String	withoutSlash = "http://www.test.example/dav/collection",
 				withSlash = withoutSlash + "/";
 		assertEquals(new URI(withSlash), URIUtils.ensureTrailingSlash(new URI(withoutSlash)));
 		assertEquals(new URI(withSlash), URIUtils.ensureTrailingSlash(new URI(withSlash)));
@@ -44,18 +44,24 @@ public class URLUtilsTest extends TestCase {
 	public void testParseURI() throws Exception {
 		// don't escape valid characters
 		String validPath = "/;:@&=$-_.+!*'(),";
-		assertEquals(new URI("https://www.test.at:123" + validPath), URIUtils.parseURI("https://www.test.at:123" + validPath));
+		assertEquals(new URI("https://www.test.example:123" + validPath), URIUtils.parseURI("https://www.test.example:123" + validPath, false));
-		assertEquals(new URI(validPath), URIUtils.parseURI(validPath));
+		assertEquals(new URI(validPath), URIUtils.parseURI(validPath, true));
 		
 		// keep literal IPv6 addresses (only in host name)
-		assertEquals(new URI("https://[1:2::1]/"), URIUtils.parseURI("https://[1:2::1]/"));
+		assertEquals(new URI("https://[1:2::1]/"), URIUtils.parseURI("https://[1:2::1]/", false));
 		
-		// ~ as home directory
+		// "~" as home directory
-		assertEquals(new URI("http://www.test.at/~user1/"), URIUtils.parseURI("http://www.test.at/~user1/"));
+		assertEquals(new URI("http://www.test.example/~user1/"), URIUtils.parseURI("http://www.test.example/~user1/", false));
-		assertEquals(new URI("http://www.test.at/~user1/"), URIUtils.parseURI("http://www.test.at/%7euser1/"));
+		assertEquals(new URI("/~user1/"), URIUtils.parseURI("/%7euser1/", true));
 		
-		// @ in directory name
+		// "@" in directory name
-		assertEquals(new URI("http://www.test.at/user@server.com/"), URIUtils.parseURI("http://www.test.at/user@server.com/"));
+		assertEquals(new URI("http://www.test.example/user@server.com/"), URIUtils.parseURI("http://www.test.example/user@server.com/", false));
-		assertEquals(new URI("http://www.test.at/user@server.com/"), URIUtils.parseURI("http://www.test.at/user%40server.com/"));
+        assertEquals(new URI("/user@server.com/"), URIUtils.parseURI("/user%40server.com/", true));
+		assertEquals(new URI("user@server.com"), URIUtils.parseURI("user%40server.com", true));
+        
+        // ":" in path names
+        assertEquals(new URI("http://www.test.example/my:cal.ics"), URIUtils.parseURI("http://www.test.example/my:cal.ics", false));
+        assertEquals(new URI("/my:cal.ics"), URIUtils.parseURI("/my%3Acal.ics", true));
+        assertEquals(new URI(null, null, "my:cal.ics", null, null), URIUtils.parseURI("my%3Acal.ics", true));
 	}
 }

app/src/androidTest/java/at/bitfire/davdroid/webdav/WebDavResourceTest.java

@@ -36,9 +36,8 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 	CloseableHttpClient httpClient;
 	
 	WebDavResource baseDAV;
-	WebDavResource simpleFile,
+	WebDavResource davAssets,
-		davCollection, davNonExistingFile, davExistingFile,
+		davCollection, davNonExistingFile, davExistingFile;
-		davInvalid;
 
 	@Override
 	protected void setUp() throws Exception {
@@ -47,14 +46,11 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 		assetMgr = getInstrumentation().getContext().getResources().getAssets();
 		
 		baseDAV = new WebDavResource(httpClient, TestConstants.roboHydra.resolve("/dav/"));
-		
+		davAssets = new WebDavResource(httpClient, TestConstants.roboHydra.resolve("assets/"));
-		simpleFile = new WebDavResource(httpClient, TestConstants.roboHydra.resolve("assets/test.random"));
-		
 		davCollection = new WebDavResource(httpClient, TestConstants.roboHydra.resolve("dav/"));
+
 		davNonExistingFile = new WebDavResource(davCollection, "collection/new.file");
 		davExistingFile = new WebDavResource(davCollection, "collection/existing.file");
-		
-		davInvalid = new WebDavResource(httpClient, TestConstants.roboHydra.resolve("dav-invalid/"));
 	}
 	
 	@Override
@@ -81,6 +77,7 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 		davCollection.propfind(HttpPropfind.Mode.CURRENT_USER_PRINCIPAL);
 		assertEquals("/dav/principals/users/test", davCollection.getCurrentUserPrincipal());
 
+        WebDavResource simpleFile = new WebDavResource(davAssets, "test.random");
 		try {
 			simpleFile.propfind(HttpPropfind.Mode.CURRENT_USER_PRINCIPAL);
 			fail();
@@ -152,6 +149,7 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 	}
 	
 	public void testGet() throws Exception {
+        WebDavResource simpleFile = new WebDavResource(davAssets, "test.random");
 		simpleFile.get("*/*");
 		@Cleanup InputStream is = assetMgr.open("test.random", AssetManager.ACCESS_STREAMING);
 		byte[] expected = IOUtils.toByteArray(is);
@@ -163,7 +161,7 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 		
 		boolean	sniWorking = false;
 		try {
-			file.get("*/*");
+			file.get("* /*");
 			sniWorking = true; 
 		} catch (SSLPeerUnverifiedException e) {
 		}
@@ -222,12 +220,14 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 	
 	/* special test */
 	
-	public void testInvalidURLs() throws Exception {
+	public void testGetSpecialURLs() throws Exception {
-		WebDavResource dav = new WebDavResource(davInvalid, "addressbooks/%7euser1/");
+        WebDavResource dav = new WebDavResource(davAssets, "member-with:colon.vcf");
-		dav.propfind(HttpPropfind.Mode.CARDDAV_COLLECTIONS);
+        try {
-		List<WebDavResource> members = dav.getMembers();
+            dav.get("*/*");
-		assertEquals(1, members.size());
+            fail();
-		assertEquals(TestConstants.ROBOHYDRA_BASE + "dav-invalid/addressbooks/~user1/My%20Contacts:1.vcf/", members.get(0).getLocation().toASCIIString());
+        } catch(NotFoundException e) {
+            assertTrue(true);
+        }
 	}
 	
 }

app/src/androidTest/robohydra/davdroid.conf

@@ -1,6 +1,5 @@
 {"plugins":[
 	"assets",
 	"redirect",
-	"dav",
+	"dav"
-	"dav-invalid"
 ]}

app/src/androidTest/robohydra/plugins/dav-invalid/index.js

@@ -1,36 +0,0 @@
-var roboHydraHeadDAV = require("../headdav");
-
-exports.getBodyParts = function(conf) {
-    return {
-        heads: [
-			/* address-book home set */
-            new RoboHydraHeadDAV({
-				path: "/dav-invalid/addressbooks/~user1/",
-				handler: function(req,res,next) {
-					if (req.method == "PROPFIND" && req.rawBody.toString().match(/addressbook-description/)) {
-						res.statusCode = 207;
-						res.write('\<?xml version="1.0" encoding="utf-8" ?>\
-							<multistatus xmlns="DAV:">\
-								<response>\
-									<href>/dav-invalid/addressbooks/~user1/My%20Contacts:1.vcf/</href>\
-									<propstat>\
-										<prop xmlns:CARD="urn:ietf:params:xml:ns:carddav">\
-											<resourcetype>\
-												<collection/>\
-												<CARD:addressbook/>\
-											</resourcetype>\
-											<CARD:addressbook-description>\
-												Address Book with dubious characters in path\
-											</CARD:addressbook-description>\
-										</prop>\
-										<status>HTTP/1.1 200 OK</status>\
-									</propstat>\
-								</response>\
-							</multistatus>\
-						');
-					}
-				}
-            })
-        ]
-    };
-};

app/src/main/java/at/bitfire/davdroid/URIUtils.java

@@ -40,14 +40,31 @@ public class URIUtils {
 	/**
 	 * Parse a received absolute/relative URL and generate a normalized URI that can be compared.
 	 * @param original	    URI to be parsed, may be absolute or relative
+     * @param mustBePath    true if it's known that original is a path (may contain ":") and not an URI, i.e. ":" is not the scheme separator
 	 * @return			    normalized URI
 	 * @throws URISyntaxException
 	 */
-	public static URI parseURI(String original) throws URISyntaxException {
+	public static URI parseURI(String original, boolean mustBePath) throws URISyntaxException {
-		URI raw = URI.create(original);
+        if (mustBePath) {
-		URI uri = new URI(raw.getScheme(), raw.getAuthority(), raw.getPath(), raw.getQuery(), raw.getFragment());
+            // may contain ":"
-		Log.v(TAG, "Normalized URL " + original + " -> " + uri.toASCIIString());
+            // case 1: "my:file"        won't be parsed by URI correctly because it would consider "my" as URI scheme
-		return uri;
+            // case 2: "path/my:file"   will be parsed by URI correctly
+            // case 3: "my:path/file"   won't be parsed by URI correctly because it would consider "my" as URI scheme
+            int idxSlash = original.indexOf('/'),
+                idxColon = original.indexOf(':');
+            if (idxColon != -1) {
+                // colon present
+                if ((idxSlash != -1) && idxSlash < idxColon)     // There's a slash, and it's before the colon → everything OK
+                    ;
+                else    // No slash before the colon; we have to put it there
+                    original = "./" + original;
+            }
+        }
+
+		URI uri = new URI(original);
+		URI normalized = new URI(uri.getScheme(), uri.getAuthority(), uri.getPath(), uri.getQuery(), uri.getFragment());
+		Log.v(TAG, "Normalized URL " + original + " -> " + normalized.toASCIIString());
+		return normalized;
 	}
 
 }

app/src/main/java/at/bitfire/davdroid/resource/RemoteCollection.java

@@ -52,7 +52,7 @@ public abstract class RemoteCollection<T extends Resource> {
 	public RemoteCollection(CloseableHttpClient httpClient, String baseURL, String user, String password, boolean preemptiveAuth) throws URISyntaxException {
 		this.httpClient = httpClient;
 		
-		collection = new WebDavResource(httpClient, URIUtils.parseURI(baseURL), user, password, preemptiveAuth);
+		collection = new WebDavResource(httpClient, URIUtils.parseURI(baseURL, false), user, password, preemptiveAuth);
 	}
 
 	

app/src/main/java/at/bitfire/davdroid/webdav/DavRedirectStrategy.java

@@ -31,7 +31,7 @@ import at.bitfire.davdroid.URIUtils;
  */
 public class DavRedirectStrategy implements RedirectStrategy {
 	private final static String TAG = "davdroid.DavRedirectStrategy";
-	final static DavRedirectStrategy INSTANCE = new DavRedirectStrategy();
+	public final static DavRedirectStrategy INSTANCE = new DavRedirectStrategy();
 	
 	protected final static String REDIRECTABLE_METHODS[] = {
 		"OPTIONS", "GET", "PUT", "DELETE"
@@ -82,12 +82,12 @@ public class DavRedirectStrategy implements RedirectStrategy {
 			return null;
 		}
 		try {
-			URI location = URIUtils.parseURI(locationHdr.getValue());
+			URI location = URIUtils.parseURI(locationHdr.getValue(), false);
 			
 			// some servers don't return absolute URLs as required by RFC 2616
 			if (!location.isAbsolute()) {
 				Log.w(TAG, "Received invalid redirection to relative URL, repairing");
-				URI originalURI = URIUtils.parseURI(request.getRequestLine().getUri());
+				URI originalURI = URIUtils.parseURI(request.getRequestLine().getUri(), false);
 				if (!originalURI.isAbsolute()) {
 					final HttpHost target = HttpClientContext.adapt(context).getTargetHost();
 					if (target != null)

app/src/main/java/at/bitfire/davdroid/webdav/TlsSniSocketFactory.java

@@ -36,7 +36,7 @@ import javax.net.ssl.SSLSocket;
 public class TlsSniSocketFactory implements LayeredConnectionSocketFactory {
 	private static final String TAG = "davdroid.SNISocketFactory";
 	
-	final static TlsSniSocketFactory INSTANCE = new TlsSniSocketFactory();
+	public final static TlsSniSocketFactory INSTANCE = new TlsSniSocketFactory();
 	
 	private final static SSLCertificateSocketFactory sslSocketFactory =
 			(SSLCertificateSocketFactory)SSLCertificateSocketFactory.getDefault(0);

app/src/main/java/at/bitfire/davdroid/webdav/WebDavResource.java

@@ -123,7 +123,7 @@ public class WebDavResource {
 		}
 	}
 	
-	WebDavResource(WebDavResource parent) {		// copy constructor: based on existing WebDavResource, reuse settings
+	public WebDavResource(WebDavResource parent) {		// copy constructor: based on existing WebDavResource, reuse settings
 		httpClient = parent.httpClient;
 		context = parent.context;
 		location = parent.location;
@@ -134,9 +134,15 @@ public class WebDavResource {
 		location = parent.location.resolve(url);
 	}
 
+    /**
+     * Creates a WebDavResource representing a member of the parent collection.
+     * @param parent Parent collection
+     * @param member File name of the member. This may contain ":" without leading "./"!
+     * @throws URISyntaxException
+     */
 	public WebDavResource(WebDavResource parent, String member) throws URISyntaxException {
 		this(parent);
-		location = parent.location.resolve(URIUtils.parseURI(member));
+		location = parent.location.resolve(URIUtils.parseURI(member, true));
 	}
 	
 	public WebDavResource(WebDavResource parent, String member, String ETag) throws URISyntaxException {
@@ -454,7 +460,7 @@ public class WebDavResource {
 		for (DavResponse singleResponse : multiStatus.response) {
 			URI href;
 			try {
-				href = location.resolve(URIUtils.parseURI(singleResponse.getHref().href));
+				href = location.resolve(URIUtils.parseURI(singleResponse.getHref().href, false));
 			} catch(Exception ex) {
 				Log.w(TAG, "Ignoring illegal member URI in multi-status response", ex);
 				continue;