From d712238700cae35fb5a730b1db8f2450a2e2cb41 Mon Sep 17 00:00:00 2001
From: rfc2822 <hirner@bitfire.at>
Date: Sun, 20 Jul 2014 14:29:52 +0200
Subject: [PATCH] Handle redirections to relative URLs correctly (see #282) +
 tests; minor GUI change

---
 res/layout/enter_credentials.xml              |  5 +-
 .../davdroid/webdav/DavRedirectStrategy.java  | 18 +++--
 .../davdroid/webdav/WebDavResource.java       |  7 +-
 test/robohydra/davdroid.conf                  |  2 +-
 .../plugins/{dav-default => dav}/index.js     |  0
 test/robohydra/plugins/redirect/index.js      | 33 ++++++++-
 test/robohydra/plugins/simple.js              | 28 +++++++
 .../at/bitfire/davdroid/test/Constants.java   | 19 +++++
 .../webdav/DavRedirectStrategyTest.java       | 73 +++++++++++++++++++
 .../webdav/{test => }/WebDavResourceTest.java | 65 ++++++-----------
 10 files changed, 193 insertions(+), 57 deletions(-)
 rename test/robohydra/plugins/{dav-default => dav}/index.js (100%)
 create mode 100644 test/robohydra/plugins/simple.js
 create mode 100644 test/src/at/bitfire/davdroid/test/Constants.java
 create mode 100644 test/src/at/bitfire/davdroid/webdav/DavRedirectStrategyTest.java
 rename test/src/at/bitfire/davdroid/webdav/{test => }/WebDavResourceTest.java (78%)

diff --git a/res/layout/enter_credentials.xml b/res/layout/enter_credentials.xml
index d49672ae..537856d9 100644
--- a/res/layout/enter_credentials.xml
+++ b/res/layout/enter_credentials.xml
@@ -26,7 +26,8 @@
         <EditText
             android:id="@+id/baseURL"
             android:layout_gravity="fill_horizontal"
-            android:hint="myserver/dav/"
+            android:hint="my.server.com"
+            android:imeOptions="flagForceAscii|actionNext"
             android:inputType="textUri"
             android:layout_width="0dp"
             android:scrollHorizontally="true"
@@ -52,6 +53,7 @@
             android:id="@+id/userName"
             android:layout_gravity="fill_horizontal"
             android:inputType="textNoSuggestions|textEmailAddress"
+            android:imeOptions="actionNext"
             android:layout_width="0dp"
             android:scrollHorizontally="true"
             android:scrollbars="horizontal"
@@ -65,6 +67,7 @@
             android:id="@+id/password"
             android:layout_gravity="fill_horizontal"
             android:inputType="textPassword"
+            android:imeOptions="actionGo"
             android:layout_width="0dp"
             android:scrollHorizontally="true"
             android:scrollbars="horizontal"
diff --git a/src/at/bitfire/davdroid/webdav/DavRedirectStrategy.java b/src/at/bitfire/davdroid/webdav/DavRedirectStrategy.java
index 502e9e6e..7b4ce172 100644
--- a/src/at/bitfire/davdroid/webdav/DavRedirectStrategy.java
+++ b/src/at/bitfire/davdroid/webdav/DavRedirectStrategy.java
@@ -5,6 +5,7 @@ import java.net.URISyntaxException;
 
 import android.util.Log;
 import ch.boye.httpclientandroidlib.Header;
+import ch.boye.httpclientandroidlib.HttpHost;
 import ch.boye.httpclientandroidlib.HttpRequest;
 import ch.boye.httpclientandroidlib.HttpResponse;
 import ch.boye.httpclientandroidlib.ProtocolException;
@@ -13,6 +14,7 @@ import ch.boye.httpclientandroidlib.client.RedirectStrategy;
 import ch.boye.httpclientandroidlib.client.methods.HttpUriRequest;
 import ch.boye.httpclientandroidlib.client.methods.RequestBuilder;
 import ch.boye.httpclientandroidlib.client.protocol.HttpClientContext;
+import ch.boye.httpclientandroidlib.client.utils.URIUtils;
 import ch.boye.httpclientandroidlib.protocol.HttpContext;
 
 /**
@@ -75,13 +77,15 @@ public class DavRedirectStrategy implements RedirectStrategy {
 			
 			// some servers don't return absolute URLs as required by RFC 2616
 			if (!location.isAbsolute()) {
-				Log.w(TAG, "Received invalid redirection with relative URL, repairing");
-				
-				// determine original URL
-				final HttpClientContext clientContext = HttpClientContext.adapt(context);
-				final URI originalURI = new URI(clientContext.getTargetHost() + request.getRequestLine().getUri());
-				
-				// determine new location relative to original URL
+				Log.w(TAG, "Received invalid redirection to relative URL, repairing");
+				URI originalURI = new URI(request.getRequestLine().getUri());
+				if (!originalURI.isAbsolute()) {
+					final HttpHost target = HttpClientContext.adapt(context).getTargetHost();
+					if (target != null)
+						originalURI = URIUtils.rewriteURI(originalURI, target);
+					else
+						return null;
+				}
 				location = originalURI.resolve(location);
 			}
 			return location;
diff --git a/src/at/bitfire/davdroid/webdav/WebDavResource.java b/src/at/bitfire/davdroid/webdav/WebDavResource.java
index 120528f3..a9b18e9f 100644
--- a/src/at/bitfire/davdroid/webdav/WebDavResource.java
+++ b/src/at/bitfire/davdroid/webdav/WebDavResource.java
@@ -55,6 +55,10 @@ import ch.boye.httpclientandroidlib.message.BasicLineParser;
 import ch.boye.httpclientandroidlib.util.EntityUtils;
 
 
+/**
+ * Represents a WebDAV resource (file or collection).
+ * This class is used for all CalDAV/CardDAV communcation.
+ */
 @ToString
 public class WebDavResource {
 	private static final String TAG = "davdroid.WebDavResource";
@@ -119,9 +123,10 @@ public class WebDavResource {
 		}
 	}
 	
-	private WebDavResource(WebDavResource parent) {		// based on existing WebDavResource, reuse settings
+	WebDavResource(WebDavResource parent) {		// copy constructor: based on existing WebDavResource, reuse settings
 		httpClient = parent.httpClient;
 		context = parent.context;
+		location = parent.location;
 	}
 
 	protected WebDavResource(WebDavResource parent, URI uri) {
diff --git a/test/robohydra/davdroid.conf b/test/robohydra/davdroid.conf
index 0a1b71f5..37807fd7 100644
--- a/test/robohydra/davdroid.conf
+++ b/test/robohydra/davdroid.conf
@@ -1,6 +1,6 @@
 {"plugins":[
 	"assets",
 	"redirect",
-	"dav-default",
+	"dav",
 	"dav-invalid"
 ]}
diff --git a/test/robohydra/plugins/dav-default/index.js b/test/robohydra/plugins/dav/index.js
similarity index 100%
rename from test/robohydra/plugins/dav-default/index.js
rename to test/robohydra/plugins/dav/index.js
diff --git a/test/robohydra/plugins/redirect/index.js b/test/robohydra/plugins/redirect/index.js
index 65a1a482..b3b994f5 100644
--- a/test/robohydra/plugins/redirect/index.js
+++ b/test/robohydra/plugins/redirect/index.js
@@ -1,10 +1,25 @@
-var RoboHydraHead = require("robohydra").heads.RoboHydraHead;
+require('../simple');
+
+var RoboHydraHead = require('robohydra').heads.RoboHydraHead;
 
 exports.getBodyParts = function(conf) {
     return {
         heads: [
+            // well-known URIs
+            new SimpleResponseHead({
+                path: '/.well-known/caldav',
+                status: 302,
+                headers: { Location: '/dav/' }
+            }),
+            new SimpleResponseHead({
+                path: '/.well-known/caldav',
+                status: 302,
+                headers: { Location: '/dav/' }
+            }),
+
+            // generic redirections
             new RoboHydraHead({
-				path: "/redirect/301",
+				path: '/redirect/301',
 				handler: function(req,res,next) {
 					res.statusCode = 301;
                     var location = req.queryParams['to'] || '/assets/test.random';
@@ -15,7 +30,7 @@ exports.getBodyParts = function(conf) {
 				}
             }),
             new RoboHydraHead({
-				path: "/redirect/302",
+				path: '/redirect/302',
 				handler: function(req,res,next) {
 					res.statusCode = 302;
                     var location = req.queryParams['to'] || '/assets/test.random';
@@ -24,7 +39,19 @@ exports.getBodyParts = function(conf) {
 					}
 					res.end();
 				}
+            }),
+
+            // special redirections
+            new SimpleResponseHead({
+                path: '/redirect/relative',
+                status: 302,
+                headers: { Location: '/new/location' }
+            }),
+            new SimpleResponseHead({
+                path: '/redirect/without-location',
+                status: 302
             })
+
         ]
     };
 };
diff --git a/test/robohydra/plugins/simple.js b/test/robohydra/plugins/simple.js
new file mode 100644
index 00000000..3d506ee2
--- /dev/null
+++ b/test/robohydra/plugins/simple.js
@@ -0,0 +1,28 @@
+var roboHydra = require("robohydra"),
+	roboHydraHeads = roboHydra.heads,
+	roboHydraHead = roboHydraHeads.RoboHydraHead;
+
+SimpleResponseHead = roboHydraHeads.roboHydraHeadType({
+	name: 'Simple HTTP Response',
+	mandatoryProperties: [ 'path', 'status' ],
+	optionalProperties: [ 'headers', 'body' ],
+
+	parentPropBuilder: function() {
+        var head = this;
+		return {
+			path: this.path,
+			handler: function(req,res,next) {
+				res.statusCode = head.status;
+                if (typeof head.headers != 'undefined')
+                    res.headers = head.headers;
+                if (typeof head.body != 'undefined')
+                    res.write(head.body);
+                else
+                    res.write();
+				res.end();
+			}
+		}
+	}
+});
+
+module.exports = SimpleResponseHead;
diff --git a/test/src/at/bitfire/davdroid/test/Constants.java b/test/src/at/bitfire/davdroid/test/Constants.java
new file mode 100644
index 00000000..e730c9e7
--- /dev/null
+++ b/test/src/at/bitfire/davdroid/test/Constants.java
@@ -0,0 +1,19 @@
+package at.bitfire.davdroid.test;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import android.util.Log;
+
+public class Constants {
+	public static final String ROBOHYDRA_BASE = "http://10.0.0.11:3000/";
+	
+	public static URI roboHydra;
+	static {
+		try {
+			roboHydra = new URI(ROBOHYDRA_BASE);
+		} catch(URISyntaxException e) {
+			Log.wtf("davdroid.test.Constants", "Invalid RoboHydra base URL");
+		}
+	}
+}
diff --git a/test/src/at/bitfire/davdroid/webdav/DavRedirectStrategyTest.java b/test/src/at/bitfire/davdroid/webdav/DavRedirectStrategyTest.java
new file mode 100644
index 00000000..25f267ce
--- /dev/null
+++ b/test/src/at/bitfire/davdroid/webdav/DavRedirectStrategyTest.java
@@ -0,0 +1,73 @@
+package at.bitfire.davdroid.webdav;
+
+import java.io.IOException;
+
+import android.test.InstrumentationTestCase;
+import at.bitfire.davdroid.test.Constants;
+import ch.boye.httpclientandroidlib.HttpResponse;
+import ch.boye.httpclientandroidlib.client.methods.HttpOptions;
+import ch.boye.httpclientandroidlib.client.methods.HttpUriRequest;
+import ch.boye.httpclientandroidlib.client.protocol.HttpClientContext;
+import ch.boye.httpclientandroidlib.impl.client.CloseableHttpClient;
+import ch.boye.httpclientandroidlib.impl.client.HttpClientBuilder;
+import ch.boye.httpclientandroidlib.protocol.HttpContext;
+
+public class DavRedirectStrategyTest extends InstrumentationTestCase {
+	
+	CloseableHttpClient httpClient;
+	DavRedirectStrategy strategy = DavRedirectStrategy.INSTANCE;
+	
+	@Override
+	protected void setUp() {
+		httpClient = HttpClientBuilder.create()
+				.useSystemProperties()
+				.disableRedirectHandling()
+				.build();
+	}
+	
+	@Override
+	protected void tearDown() throws IOException {
+		httpClient.close();
+	}
+	
+	
+	// happy cases
+	
+	public void testNonRedirection() throws Exception {
+		HttpUriRequest request = new HttpOptions(Constants.roboHydra);
+		HttpResponse response = httpClient.execute(request);
+		assertFalse(strategy.isRedirected(request, response, null));
+	}
+	
+	public void testDefaultRedirection() throws Exception {
+		final String newLocation = "/new-location";
+		
+		HttpContext context = HttpClientContext.create();
+		HttpUriRequest request = new HttpOptions(Constants.roboHydra.resolve("redirect/301?to=" + newLocation));
+		HttpResponse response = httpClient.execute(request, context);
+		assertTrue(strategy.isRedirected(request, response, context));
+		
+		HttpUriRequest redirected = strategy.getRedirect(request, response, context);
+		assertEquals(Constants.roboHydra.resolve(newLocation), redirected.getURI());
+	}
+	
+	
+	// error cases
+	
+	public void testMissingLocation() throws Exception {
+		HttpContext context = HttpClientContext.create();
+		HttpUriRequest request = new HttpOptions(Constants.roboHydra.resolve("redirect/without-location"));
+		HttpResponse response = httpClient.execute(request, context);
+		assertFalse(strategy.isRedirected(request, response, context));
+	}
+	
+	public void testRelativeLocation() throws Exception {
+		HttpContext context = HttpClientContext.create();
+		HttpUriRequest request = new HttpOptions(Constants.roboHydra.resolve("redirect/relative"));
+		HttpResponse response = httpClient.execute(request, context);
+		assertTrue(strategy.isRedirected(request, response, context));
+		
+		HttpUriRequest redirected = strategy.getRedirect(request, response, context);
+		assertEquals(Constants.roboHydra.resolve("/new/location"), redirected.getURI());
+	}
+}
diff --git a/test/src/at/bitfire/davdroid/webdav/test/WebDavResourceTest.java b/test/src/at/bitfire/davdroid/webdav/WebDavResourceTest.java
similarity index 78%
rename from test/src/at/bitfire/davdroid/webdav/test/WebDavResourceTest.java
rename to test/src/at/bitfire/davdroid/webdav/WebDavResourceTest.java
index e1fc9954..0203d19f 100644
--- a/test/src/at/bitfire/davdroid/webdav/test/WebDavResourceTest.java
+++ b/test/src/at/bitfire/davdroid/webdav/WebDavResourceTest.java
@@ -5,7 +5,7 @@
  * which accompanies this distribution, and is available at
  * http://www.gnu.org/licenses/gpl.html
  ******************************************************************************/
-package at.bitfire.davdroid.webdav.test;
+package at.bitfire.davdroid.webdav;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -22,6 +22,7 @@ import org.apache.commons.io.IOUtils;
 
 import android.content.res.AssetManager;
 import android.test.InstrumentationTestCase;
+import at.bitfire.davdroid.test.Constants;
 import at.bitfire.davdroid.webdav.DavException;
 import at.bitfire.davdroid.webdav.DavHttpClient;
 import at.bitfire.davdroid.webdav.DavMultiget;
@@ -37,12 +38,14 @@ import ch.boye.httpclientandroidlib.impl.client.CloseableHttpClient;
 // tests require running robohydra!
 
 public class WebDavResourceTest extends InstrumentationTestCase {
-	static final String ROBOHYDRA_BASE = "http://10.0.0.11:3000/";
 	static byte[] SAMPLE_CONTENT = new byte[] { 1, 2, 3, 4, 5 };
 	
+	final static String PATH_SIMPLE_FILE = "collection/new.file";
+	
 	AssetManager assetMgr;
 	CloseableHttpClient httpClient;
 	
+	WebDavResource baseDAV;
 	WebDavResource simpleFile,
 		davCollection, davNonExistingFile, davExistingFile,
 		davInvalid;
@@ -53,57 +56,35 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 				
 		assetMgr = getInstrumentation().getContext().getResources().getAssets();
 		
-		simpleFile = new WebDavResource(httpClient, new URI(ROBOHYDRA_BASE + "assets/test.random"));
+		baseDAV = new WebDavResource(httpClient, Constants.roboHydra.resolve("/dav/"));
 		
-		davCollection = new WebDavResource(httpClient, new URI(ROBOHYDRA_BASE + "dav/"));
+		simpleFile = new WebDavResource(httpClient, new URI(Constants.ROBOHYDRA_BASE + "assets/test.random"));
+		
+		davCollection = new WebDavResource(httpClient, new URI(Constants.ROBOHYDRA_BASE + "dav/"));
 		davNonExistingFile = new WebDavResource(davCollection, "collection/new.file");
 		davExistingFile = new WebDavResource(davCollection, "collection/existing.file");
 		
-		davInvalid = new WebDavResource(httpClient, new URI(ROBOHYDRA_BASE + "dav-invalid/"));
+		davInvalid = new WebDavResource(httpClient, new URI(Constants.ROBOHYDRA_BASE + "dav-invalid/"));
 	}
 	
 	@Override
-	protected void tearDown() throws Exception {
+	protected void tearDown() throws IOException {
 		httpClient.close();
 	}
 
 
-	/* test resource name handling */
-
-	public void testGetName() {
-		// collection names should have a trailing slash
-		assertEquals("dav", davCollection.getName());
-		// but non-collection names shouldn't
-		assertEquals("test.random", simpleFile.getName());
-	}
-	
-	public void testTrailingSlash() throws URISyntaxException {
-		// collections should have a trailing slash
-		assertEquals("/dav/", davCollection.getLocation().getPath());
-		// but non-collection members shouldn't
-		assertEquals("/assets/test.random", simpleFile.getLocation().getPath());
-	}
-	
-	
 	/* test feature detection */
 	
 	public void testOptions() throws URISyntaxException, IOException, HttpException {
-		String[]	davMethods = new String[] { "PROPFIND", "GET", "PUT", "DELETE" },
+		String[]	davMethods = new String[] { "PROPFIND", "GET", "PUT", "DELETE", "REPORT" },
 					davCapabilities = new String[] { "addressbook", "calendar-access" };
 		
-		// server without DAV
-		simpleFile.options();
-		for (String method : davMethods)
-			assertFalse(simpleFile.supportsMethod(method));
-		for (String capability : davCapabilities)
-			assertFalse(simpleFile.supportsDAV(capability));
-		
-		// server with DAV
-		davCollection.options();
+		WebDavResource capable = new WebDavResource(baseDAV);
+		capable.options();
 		for (String davMethod : davMethods)
-			assert(davCollection.supportsMethod(davMethod));
+			assert(capable.supportsMethod(davMethod));
 		for (String capability : davCapabilities)
-			assert(davCollection.supportsDAV(capability));
+			assert(capable.supportsDAV(capability));
 	}
 
 	public void testPropfindCurrentUserPrincipal() throws IOException, HttpException, DavException {
@@ -156,15 +137,11 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 	
 	/* test normal HTTP/WebDAV */
 	
-	public void testRedirections() throws URISyntaxException, IOException, DavException, HttpException {
+	public void testPropfindRedirection() throws URISyntaxException, IOException, DavException, HttpException {
 		// PROPFIND redirection
-		WebDavResource redirection = new WebDavResource(httpClient, new URI(ROBOHYDRA_BASE + "redirect/301?to=/dav/"));
-		redirection.propfind(Mode.CURRENT_USER_PRINCIPAL);
-		assertEquals("/dav/", redirection.getLocation().getPath());
-		
-		// normal GET redirection
-		redirection = new WebDavResource(httpClient, new URI(ROBOHYDRA_BASE + "redirect/301"));
-		redirection.get();
+		WebDavResource redirected = new WebDavResource(baseDAV, "/redirect/301?to=/dav/");
+		redirected.propfind(Mode.CURRENT_USER_PRINCIPAL);
+		assertEquals("/dav/", redirected.getLocation().getPath());
 	}
 	
 	public void testGet() throws URISyntaxException, IOException, HttpException, DavException {
@@ -243,7 +220,7 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 		dav.propfind(HttpPropfind.Mode.MEMBERS_COLLECTIONS);
 		List<WebDavResource> members = dav.getMembers();
 		assertEquals(2, members.size());
-		assertEquals(ROBOHYDRA_BASE + "dav/addressbooks/user%40domain/My%20Contacts%3a1.vcf/", members.get(0).getLocation().toString());
+		assertEquals(Constants.ROBOHYDRA_BASE + "dav/addressbooks/user%40domain/My%20Contacts%3a1.vcf/", members.get(0).getLocation().toString());
 		assertEquals("HTTPS://example.com/user%40domain/absolute-url.vcf", members.get(1).getLocation().toString());
 	}