From c753dcaa8ef54e8db846d0a4eca6bc0e5d84fed9 Mon Sep 17 00:00:00 2001 From: rfc2822 Date: Sat, 9 Nov 2013 01:13:38 +0100 Subject: [PATCH] RoboHydra tests, URL sanitizing * RoboHydra tests * intelligent URL sanitizing (fixes #58, fixes #49, see issue #11, should fix #45) --- src/at/bitfire/davdroid/URIUtils.java | 40 +++++++++++++++---- .../syncadapter/EnterCredentialsFragment.java | 6 ++- .../davdroid/webdav/WebDavResource.java | 6 +-- test/robohydra/davdroid.conf | 3 +- test/robohydra/plugins/dav-invalid/index.js | 19 ++++++++- .../bitfire/davdroid/test/URIUtilsTest.java | 16 ++------ .../webdav/test/WebDavResourceTest.java | 24 ++++++++++- 7 files changed, 85 insertions(+), 29 deletions(-) diff --git a/src/at/bitfire/davdroid/URIUtils.java b/src/at/bitfire/davdroid/URIUtils.java index e937948e..1c220076 100644 --- a/src/at/bitfire/davdroid/URIUtils.java +++ b/src/at/bitfire/davdroid/URIUtils.java @@ -3,21 +3,47 @@ package at.bitfire.davdroid; import java.net.URI; import java.net.URISyntaxException; +import android.annotation.SuppressLint; +import android.util.Log; + +@SuppressLint("DefaultLocale") public class URIUtils { + private static final String TAG = "davdroid.URIUtils"; + public static boolean isSame(URI a, URI b) { try { - a = new URI(a.getScheme(), null, a.getHost(), a.getPort(), a.getPath(), a.getQuery(), a.getFragment()); - b = new URI(b.getScheme(), null, b.getHost(), b.getPort(), b.getPath(), b.getQuery(), b.getFragment()); + a = new URI(a.getScheme(), null, a.getHost(), a.getPort(), sanitize(a.getPath()), sanitize(a.getQuery()), null); + b = new URI(b.getScheme(), null, b.getHost(), b.getPort(), sanitize(b.getPath()), sanitize(b.getQuery()), null); return a.equals(b); } catch (URISyntaxException e) { return false; } } - - public static URI resolve(URI parent, String member) { - if (!member.startsWith("/") && !member.startsWith("http:") && !member.startsWith("https:")) - member = "./" + member; + + // handles invalid URLs/paths as good as possible + public static String sanitize(String original) { + if (original == null) + return null; - return parent.resolve(member); + String url = original; + + // ":" is reserved as scheme/port separator, but we assume http:// and https:// URLs only + // and will encode ":" in URLs without one of these schemata + if (!url.toLowerCase().startsWith("http://") && // ":" is valid scheme separator + !url.toLowerCase().startsWith("https://") && // ":" is valid scheme separator + !url.startsWith("//")) // ":" may be valid port separator + url = url.replace(":", "%3A"); // none of these -> ":" is not used for reserved purpose -> must be encoded + + // rewrite reserved characters: + // "@" should be used for user name/password, but this case shouldn't appear in our URLs + // rewrite unsafe characters, too: + // " ", "<", ">", """, "#", "{", "}", "|", "\", "^", "~", "["], "]", "`" + // do not rewrite "%" because we assume that URLs should be already encoded correctly + for (char c : new char[] { '@', ' ', '<', '>', '"', '#', '{', '}', '|', '\\', '^', '~', '[', ']', '`' }) + url = url.replace(String.valueOf(c), "%" + Integer.toHexString(c)); + + if (!url.equals(original)) + Log.w(TAG, "Tried to repair invalid URL/URL path: " + original + " -> " + url); + return url; } } diff --git a/src/at/bitfire/davdroid/syncadapter/EnterCredentialsFragment.java b/src/at/bitfire/davdroid/syncadapter/EnterCredentialsFragment.java index d5c26c37..2ec8b796 100644 --- a/src/at/bitfire/davdroid/syncadapter/EnterCredentialsFragment.java +++ b/src/at/bitfire/davdroid/syncadapter/EnterCredentialsFragment.java @@ -26,7 +26,9 @@ import android.widget.CheckBox; import android.widget.EditText; import android.widget.Spinner; import android.widget.TextView; +import android.widget.Toast; import at.bitfire.davdroid.R; +import at.bitfire.davdroid.URIUtils; public class EnterCredentialsFragment extends Fragment implements TextWatcher { String protocol; @@ -102,9 +104,10 @@ public class EnterCredentialsFragment extends Fragment implements TextWatcher { void queryServer() { FragmentTransaction ft = getFragmentManager().beginTransaction(); - String host_path = editBaseURL.getText().toString(); + String host_path = URIUtils.sanitize(editBaseURL.getText().toString()); Bundle args = new Bundle(); + args.putString(QueryServerDialogFragment.EXTRA_BASE_URL, protocol + host_path); args.putString(QueryServerDialogFragment.EXTRA_USER_NAME, editUserName.getText().toString()); args.putString(QueryServerDialogFragment.EXTRA_PASSWORD, editPassword.getText().toString()); @@ -122,6 +125,7 @@ public class EnterCredentialsFragment extends Fragment implements TextWatcher { public void onPrepareOptionsMenu(Menu menu) { boolean ok = editBaseURL.getText().length() > 0 && + !editBaseURL.getText().toString().startsWith("/") && // host name required editUserName.getText().length() > 0 && editPassword.getText().length() > 0; diff --git a/src/at/bitfire/davdroid/webdav/WebDavResource.java b/src/at/bitfire/davdroid/webdav/WebDavResource.java index 3e3a05cb..87bae3f1 100644 --- a/src/at/bitfire/davdroid/webdav/WebDavResource.java +++ b/src/at/bitfire/davdroid/webdav/WebDavResource.java @@ -128,7 +128,7 @@ public class WebDavResource { } public WebDavResource(WebDavResource parent, String member) { - location = URIUtils.resolve(parent.location, member); + location = parent.location.resolve(URIUtils.sanitize(member)); client = parent.client; } @@ -285,7 +285,7 @@ public class WebDavResource { multiget.hrefs = new ArrayList(names.length); for (String name : names) - multiget.hrefs.add(new DavHref(URIUtils.resolve(location, name).getPath())); + multiget.hrefs.add(new DavHref(location.resolve(name).getPath())); Serializer serializer = new Persister(); StringWriter writer = new StringWriter(); @@ -371,7 +371,7 @@ public class WebDavResource { for (DavResponse singleResponse : multistatus.response) { URI href; try { - href = URIUtils.resolve(location, singleResponse.getHref().href); + href = location.resolve(URIUtils.sanitize(singleResponse.getHref().href)); } catch(IllegalArgumentException ex) { Log.w(TAG, "Ignoring illegal member URI in multi-status response", ex); continue; diff --git a/test/robohydra/davdroid.conf b/test/robohydra/davdroid.conf index 441ec99a..0a1b71f5 100644 --- a/test/robohydra/davdroid.conf +++ b/test/robohydra/davdroid.conf @@ -1,5 +1,6 @@ {"plugins":[ "assets", "redirect", - "dav-default" + "dav-default", + "dav-invalid" ]} diff --git a/test/robohydra/plugins/dav-invalid/index.js b/test/robohydra/plugins/dav-invalid/index.js index b1c41a47..dba0c28d 100644 --- a/test/robohydra/plugins/dav-invalid/index.js +++ b/test/robohydra/plugins/dav-invalid/index.js @@ -12,7 +12,7 @@ exports.getBodyParts = function(conf) { res.write('\\ \ \ - /dav/addressbooks/user@domain/My Contacts.vcf/\ + /dav/addressbooks/user@domain/My Contacts:1.vcf/\ \ \ \ @@ -20,7 +20,22 @@ exports.getBodyParts = function(conf) { \ \ \ - Address Book with @ and space in URL\ + Address Book with dubious characters in path\ + \ + \ + HTTP/1.1 200 OK\ + \ + \ + \ + HTTPS://example.com/user@domain/absolute-url.vcf\ + \ + \ + \ + \ + \ + \ + \ + Address Book with absolute URL\ \ \ HTTP/1.1 200 OK\ diff --git a/test/src/at/bitfire/davdroid/test/URIUtilsTest.java b/test/src/at/bitfire/davdroid/test/URIUtilsTest.java index a77a0e89..558523a6 100644 --- a/test/src/at/bitfire/davdroid/test/URIUtilsTest.java +++ b/test/src/at/bitfire/davdroid/test/URIUtilsTest.java @@ -22,18 +22,8 @@ public class URIUtilsTest extends InstrumentationTestCase { assertTrue(URIUtils.isSame(new URI(ROOT_URI + "my@email/"), new URI(ROOT_URI + "my%40email/"))); } - public void testResolve() { - // resolve absolute URL - assertEquals(ROOT_URI + "file", URIUtils.resolve(baseURI, "/file").toString()); - - // resolve relative URL (default case) - assertEquals(BASE_URI + "file", URIUtils.resolve(baseURI, "file").toString()); - - // resolve relative URL with special characters - assertEquals(BASE_URI + "fi:le", URIUtils.resolve(baseURI, "fi:le").toString()); - assertEquals(BASE_URI + "fi@le", URIUtils.resolve(baseURI, "fi@le").toString()); - - // resolve URL with other schema - assertEquals("https://server", URIUtils.resolve(baseURI, "https://server").toString()); + public void testSanitize() { + assertEquals("/my%40email.com/dir", URIUtils.sanitize("/my@email.com/dir")); + assertEquals("my%3Afile.vcf", URIUtils.sanitize("my:file.vcf")); } } diff --git a/test/src/at/bitfire/davdroid/webdav/test/WebDavResourceTest.java b/test/src/at/bitfire/davdroid/webdav/test/WebDavResourceTest.java index 7653e100..d2d506d2 100644 --- a/test/src/at/bitfire/davdroid/webdav/test/WebDavResourceTest.java +++ b/test/src/at/bitfire/davdroid/webdav/test/WebDavResourceTest.java @@ -3,6 +3,7 @@ package at.bitfire.davdroid.webdav.test; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.util.List; import org.apache.commons.io.IOUtils; import org.apache.http.HttpException; @@ -23,7 +24,8 @@ public class WebDavResourceTest extends InstrumentationTestCase { AssetManager assetMgr; WebDavResource simpleFile, - davCollection, davNonExistingFile, davExistingFile; + davCollection, davNonExistingFile, davExistingFile, + davInvalid; @Override protected void setUp() throws Exception { @@ -34,6 +36,8 @@ public class WebDavResourceTest extends InstrumentationTestCase { davCollection = new WebDavResource(new URI(ROBOHYDRA_BASE + "dav"), true); davNonExistingFile = new WebDavResource(davCollection, "collection/new.file"); davExistingFile = new WebDavResource(davCollection, "collection/existing.file"); + + davInvalid = new WebDavResource(new URI(ROBOHYDRA_BASE + "dav-invalid"), true); } @@ -117,7 +121,7 @@ public class WebDavResourceTest extends InstrumentationTestCase { } - /* test normal HTTP */ + /* test normal HTTP/WebDAV */ public void testDontFollowRedirections() throws URISyntaxException, IOException { WebDavResource redirection = new WebDavResource(new URI(ROBOHYDRA_BASE + "redirect"), false); @@ -175,4 +179,20 @@ public class WebDavResourceTest extends InstrumentationTestCase { } fail(); } + + + /* test CalDAV/CardDAV */ + + + /* special test */ + + public void testInvalidURLs() throws IOException, HttpException { + WebDavResource dav = new WebDavResource(davInvalid, "addressbooks/user%40domain/"); + dav.propfind(HttpPropfind.Mode.MEMBERS_COLLECTIONS); + List members = dav.getMembers(); + assertEquals(2, members.size()); + assertEquals(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()); + } + }