From 825ef70cef575e6467aaf446d3479af4ef950567 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Fri, 7 Apr 2017 14:08:59 +0200 Subject: [PATCH 1/3] Allow the usage of OwnCloudVersion instances for invalid server versions, and removed some really old or never used endpoint paths --- .../android/lib/common/OwnCloudClient.java | 8 +- .../lib/common/accounts/AccountUtils.java | 48 +------- .../status/GetRemoteStatusOperation.java | 38 +++--- .../lib/resources/status/OwnCloudVersion.java | 115 +++++++++--------- 4 files changed, 81 insertions(+), 128 deletions(-) diff --git a/src/com/owncloud/android/lib/common/OwnCloudClient.java b/src/com/owncloud/android/lib/common/OwnCloudClient.java index 3cd60cbd..dc3ea126 100644 --- a/src/com/owncloud/android/lib/common/OwnCloudClient.java +++ b/src/com/owncloud/android/lib/common/OwnCloudClient.java @@ -295,8 +295,6 @@ public class OwnCloudClient extends HttpClient { } if (destination != null) { int suffixIndex = locationStr.lastIndexOf( - (mCredentials instanceof OwnCloudBearerCredentials) ? - AccountUtils.ODAV_PATH : AccountUtils.WEBDAV_PATH_4_0 ); String redirectionBase = locationStr.substring(0, suffixIndex); @@ -352,11 +350,7 @@ public class OwnCloudClient extends HttpClient { } public Uri getWebdavUri() { - if (mCredentials instanceof OwnCloudBearerCredentials) { - return Uri.parse(mBaseUri + AccountUtils.ODAV_PATH); - } else { - return Uri.parse(mBaseUri + AccountUtils.WEBDAV_PATH_4_0); - } + return Uri.parse(mBaseUri + AccountUtils.WEBDAV_PATH_4_0); } /** diff --git a/src/com/owncloud/android/lib/common/accounts/AccountUtils.java b/src/com/owncloud/android/lib/common/accounts/AccountUtils.java index 8cb1f115..b575d31d 100644 --- a/src/com/owncloud/android/lib/common/accounts/AccountUtils.java +++ b/src/com/owncloud/android/lib/common/accounts/AccountUtils.java @@ -1,5 +1,5 @@ /* ownCloud Android Library is available under MIT license - * Copyright (C) 2016 ownCloud GmbH. + * Copyright (C) 2017 ownCloud GmbH. * Copyright (C) 2012 Bartek Przybylski * * Permission is hereby granted, free of charge, to any person obtaining a copy @@ -47,43 +47,9 @@ public class AccountUtils { private static final String TAG = AccountUtils.class.getSimpleName(); - public static final String WEBDAV_PATH_1_2 = "/webdav/owncloud.php"; - public static final String WEBDAV_PATH_2_0 = "/files/webdav.php"; public static final String WEBDAV_PATH_4_0 = "/remote.php/webdav"; - public static final String ODAV_PATH = "/remote.php/odav"; - private static final String SAML_SSO_PATH = "/remote.php/webdav"; - public static final String CARDDAV_PATH_2_0 = "/apps/contacts/carddav.php"; - public static final String CARDDAV_PATH_4_0 = "/remote/carddav.php"; public static final String STATUS_PATH = "/status.php"; - /** - * Returns the proper URL path to access the WebDAV interface of an ownCloud server, - * according to its version and the authorization method used. - * - * @param supportsOAuth If true, access with OAuth 2 authorization is considered. - * @param supportsSamlSso If true, and supportsOAuth is false, access with SAML-based single-sign-on is considered. - * @return WebDAV path for given OC version, null if OC version unknown - * @param version Version of ownCloud server. - */ - public static String getWebdavPath(OwnCloudVersion version, boolean supportsOAuth, boolean supportsSamlSso) { - if (version != null) { - if (supportsOAuth) { - return ODAV_PATH; - } - if (supportsSamlSso) { - return SAML_SSO_PATH; - } - if (version.compareTo(OwnCloudVersion.owncloud_v4) >= 0) - return WEBDAV_PATH_4_0; - if (version.compareTo(OwnCloudVersion.owncloud_v3) >= 0 - || version.compareTo(OwnCloudVersion.owncloud_v2) >= 0) - return WEBDAV_PATH_2_0; - if (version.compareTo(OwnCloudVersion.owncloud_v1) >= 0) - return WEBDAV_PATH_1_2; - } - return null; - } - /** * Constructs full url to host and webdav resource basing on host version * @@ -97,16 +63,10 @@ public class AccountUtils { public static String constructFullURLForAccount(Context context, Account account) throws AccountNotFoundException { AccountManager ama = AccountManager.get(context); String baseurl = ama.getUserData(account, Constants.KEY_OC_BASE_URL); - String version = ama.getUserData(account, Constants.KEY_OC_VERSION); - boolean supportsOAuth = (ama.getUserData(account, Constants.KEY_SUPPORTS_OAUTH2) != null); - boolean supportsSamlSso = (ama.getUserData(account, Constants.KEY_SUPPORTS_SAML_WEB_SSO) != null); - OwnCloudVersion ver = new OwnCloudVersion(version); - String webdavpath = getWebdavPath(ver, supportsOAuth, supportsSamlSso); - - if (baseurl == null || webdavpath == null) + if (baseurl == null) { throw new AccountNotFoundException(account, "Account not found", null); - - return baseurl + webdavpath; + } + return baseurl + WEBDAV_PATH_4_0; } /** diff --git a/src/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.java b/src/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.java index d488274f..4b87d73c 100644 --- a/src/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.java +++ b/src/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.java @@ -115,30 +115,26 @@ public class GetRemoteStatusOperation extends RemoteOperation { } else { String version = json.getString(NODE_VERSION); OwnCloudVersion ocVersion = new OwnCloudVersion(version); - if (!ocVersion.isVersionValid()) { + /// the version object will be returned even if the version is invalid, no error code; + /// every app will decide how to act if (ocVersion.isVersionValid() == false) + + if (isRedirectToNonSecureConnection) { mLatestResult = new RemoteOperationResult( - RemoteOperationResult.ResultCode.BAD_OC_VERSION); - + RemoteOperationResult.ResultCode. + OK_REDIRECT_TO_NON_SECURE_CONNECTION + ); } else { - // success - if (isRedirectToNonSecureConnection) { - mLatestResult = new RemoteOperationResult( - RemoteOperationResult.ResultCode. - OK_REDIRECT_TO_NON_SECURE_CONNECTION - ); - } else { - mLatestResult = new RemoteOperationResult( - baseUrlSt.startsWith("https://") ? - RemoteOperationResult.ResultCode.OK_SSL : - RemoteOperationResult.ResultCode.OK_NO_SSL - ); - } - - ArrayList data = new ArrayList(); - data.add(ocVersion); - mLatestResult.setData(data); - retval = true; + mLatestResult = new RemoteOperationResult( + baseUrlSt.startsWith("https://") ? + RemoteOperationResult.ResultCode.OK_SSL : + RemoteOperationResult.ResultCode.OK_NO_SSL + ); } + + ArrayList data = new ArrayList(); + data.add(ocVersion); + mLatestResult.setData(data); + retval = true; } } else { diff --git a/src/com/owncloud/android/lib/resources/status/OwnCloudVersion.java b/src/com/owncloud/android/lib/resources/status/OwnCloudVersion.java index 76ae4710..a2d830e7 100644 --- a/src/com/owncloud/android/lib/resources/status/OwnCloudVersion.java +++ b/src/com/owncloud/android/lib/resources/status/OwnCloudVersion.java @@ -26,16 +26,6 @@ package com.owncloud.android.lib.resources.status; public class OwnCloudVersion implements Comparable { - public static final OwnCloudVersion owncloud_v1 = new OwnCloudVersion( - 0x01000000); - public static final OwnCloudVersion owncloud_v2 = new OwnCloudVersion( - 0x02000000); - public static final OwnCloudVersion owncloud_v3 = new OwnCloudVersion( - 0x03000000); - public static final OwnCloudVersion owncloud_v4 = new OwnCloudVersion( - 0x04000000); - public static final OwnCloudVersion owncloud_v4_5 = new OwnCloudVersion( - 0x04050000); public static final int MINIMUN_VERSION_FOR_CHUNKED_UPLOADS = 0x04050000; // 4.5 @@ -56,7 +46,7 @@ public class OwnCloudVersion implements Comparable { private static final int MINIMUM_VERSION_WITH_SESSION_MONITORING = 0x09010000; // 9.1 private static final int MINIMUM_VERSION_WITH_SESSION_MONITORING_WORKING_IN_PREEMPTIVE_MODE = 0x09010301; - // 9.1.3.1, final 9.1.3: https://github.com/owncloud/core/commit/f9a867b70c217463289a741d4d26079eb2a80dfd + // 9.1.3.1, final 9.1.3: https://github.com/owncloud/core/commit/f9a867b70c217463289a741d4d26079eb2a80dfd private static final int MAX_DOTS = 3; @@ -67,37 +57,47 @@ public class OwnCloudVersion implements Comparable { private int mVersion; private boolean mIsValid; - protected OwnCloudVersion(int version) { + /** + * @deprecated Will be removed in version 1.0 of the library. + */ + private OwnCloudVersion(int version) { mVersion = version; mIsValid = true; } - - public OwnCloudVersion(String version){ - mVersion = 0; - mIsValid = false; - int countDots = version.length() - version.replace(".", "").length(); - // Complete the version. Version must have 3 dots - for (int i = countDots; i < MAX_DOTS; i++) { - version = version + ".0"; - } - - parseVersion(version); + public OwnCloudVersion(String version) { + mVersion = 0; + mIsValid = false; + int countDots = version.length() - version.replace(".", "").length(); + + // Complete the version. Version must have 3 dots + for (int i = countDots; i < MAX_DOTS; i++) { + version = version + ".0"; + } + + parseVersion(version); } - + public String toString() { - String versionToString = String.valueOf((mVersion >> (8*MAX_DOTS)) % 256); - for (int i = MAX_DOTS - 1; i >= 0; i-- ) { - versionToString = versionToString + "." + String.valueOf((mVersion >> (8*i)) % 256); - } + String versionToString = String.valueOf((mVersion >> (8 * MAX_DOTS)) % 256); + for (int i = MAX_DOTS - 1; i >= 0; i--) { + versionToString = versionToString + "." + String.valueOf((mVersion >> (8 * i)) % 256); + } + if (!mIsValid) { + versionToString += " INVALID"; + } return versionToString; } - + public String getVersion() { - return toString(); + if (mIsValid) { + return toString(); + } else { + return "0.0.0"; + } } - + public boolean isVersionValid() { return mIsValid; } @@ -105,34 +105,37 @@ public class OwnCloudVersion implements Comparable { @Override public int compareTo(OwnCloudVersion another) { return another.mVersion == mVersion ? 0 - : another.mVersion < mVersion ? 1 : -1; + : another.mVersion < mVersion ? 1 : -1; } private void parseVersion(String version) { - try { - mVersion = getParsedVersion(version); - mIsValid = true; - - } catch (Exception e) { - mIsValid = false; + try { + mVersion = getParsedVersion(version); + mIsValid = true; + + } catch (Exception e) { + mIsValid = false; + // if invalid, the instance will respond as if server is 8.1, minimum with capabilities API, + // and "dead" : https://github.com/owncloud/core/wiki/Maintenance-and-Release-Schedule + mVersion = MINIMUM_VERSION_CAPABILITIES_API; } } - + private int getParsedVersion(String version) throws NumberFormatException { - int versionValue = 0; + int versionValue = 0; - // get only numeric part - version = version.replaceAll("[^\\d.]", ""); + // get only numeric part + version = version.replaceAll("[^\\d.]", ""); - String[] nums = version.split("\\."); - for (int i = 0; i < nums.length && i <= MAX_DOTS; i++) { - versionValue += Integer.parseInt(nums[i]); - if (i < nums.length - 1) { - versionValue = versionValue << 8; - } - } + String[] nums = version.split("\\."); + for (int i = 0; i < nums.length && i <= MAX_DOTS; i++) { + versionValue += Integer.parseInt(nums[i]); + if (i < nums.length - 1) { + versionValue = versionValue << 8; + } + } - return versionValue; + return versionValue; } @@ -141,7 +144,7 @@ public class OwnCloudVersion implements Comparable { } public boolean isSharedSupported() { - return (mVersion >= MINIMUM_VERSION_FOR_SHARING_API); + return (mVersion >= MINIMUM_VERSION_FOR_SHARING_API); } public boolean isVersionWithForbiddenCharacters() { @@ -152,7 +155,7 @@ public class OwnCloudVersion implements Comparable { return (mVersion >= MINIMUM_SERVER_VERSION_FOR_REMOTE_THUMBNAILS); } - public boolean isAfter8Version(){ + public boolean isAfter8Version() { return (mVersion >= VERSION_8); } @@ -160,8 +163,8 @@ public class OwnCloudVersion implements Comparable { return (mVersion >= MINIMUM_VERSION_FOR_SEARCHING_USERS); } - public boolean isVersionWithCapabilitiesAPI(){ - return (mVersion>= MINIMUM_VERSION_CAPABILITIES_API); + public boolean isVersionWithCapabilitiesAPI() { + return (mVersion >= MINIMUM_VERSION_CAPABILITIES_API); } public boolean isNotReshareableFederatedSupported() { @@ -177,11 +180,11 @@ public class OwnCloudVersion implements Comparable { * mode of basic authentication is disabled. This changes in OC 9.1.3, where preemptive mode is compatible * with session tracking again. * - * @return True for every version before 9.1 and from 9.1.3, false otherwise + * @return True for every version before 9.1 and from 9.1.3, false otherwise */ public boolean isPreemptiveAuthenticationPreferred() { return ( - (mVersion < MINIMUM_VERSION_WITH_SESSION_MONITORING) || + (mVersion < MINIMUM_VERSION_WITH_SESSION_MONITORING) || (mVersion >= MINIMUM_VERSION_WITH_SESSION_MONITORING_WORKING_IN_PREEMPTIVE_MODE) ); } From f42a9f8c61fa3c2025da67d5971035f361f4a755 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Tue, 11 Apr 2017 10:08:28 +0200 Subject: [PATCH 2/3] Constants and comments after crossed review --- .../resources/status/GetRemoteStatusOperation.java | 14 ++++++++------ .../lib/resources/status/OwnCloudVersion.java | 7 ++++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.java b/src/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.java index 4b87d73c..10e1f1b7 100644 --- a/src/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.java +++ b/src/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.java @@ -63,6 +63,8 @@ public class GetRemoteStatusOperation extends RemoteOperation { private static final String NODE_INSTALLED = "installed"; private static final String NODE_VERSION = "version"; + private static final String HTTPS_PREFIX = "https://"; + private static final String HTTP_PREFIX = "http://"; private RemoteOperationResult mLatestResult; private Context mContext; @@ -93,8 +95,8 @@ public class GetRemoteStatusOperation extends RemoteOperation { && !mLatestResult.isSuccess()) { isRedirectToNonSecureConnection |= ( - baseUrlSt.startsWith("https://") && - redirectedLocation.startsWith("http://") + baseUrlSt.startsWith(HTTPS_PREFIX) && + redirectedLocation.startsWith(HTTP_PREFIX) ); get.releaseConnection(); get = new GetMethod(redirectedLocation); @@ -125,7 +127,7 @@ public class GetRemoteStatusOperation extends RemoteOperation { ); } else { mLatestResult = new RemoteOperationResult( - baseUrlSt.startsWith("https://") ? + baseUrlSt.startsWith(HTTPS_PREFIX) ? RemoteOperationResult.ResultCode.OK_SSL : RemoteOperationResult.ResultCode.OK_NO_SSL ); @@ -180,15 +182,15 @@ public class GetRemoteStatusOperation extends RemoteOperation { return new RemoteOperationResult(RemoteOperationResult.ResultCode.NO_NETWORK_CONNECTION); } String baseUriStr = client.getBaseUri().toString(); - if (baseUriStr.startsWith("http://") || baseUriStr.startsWith("https://")) { + if (baseUriStr.startsWith(HTTP_PREFIX) || baseUriStr.startsWith(HTTPS_PREFIX)) { tryConnection(client); } else { - client.setBaseUri(Uri.parse("https://" + baseUriStr)); + client.setBaseUri(Uri.parse(HTTPS_PREFIX + baseUriStr)); boolean httpsSuccess = tryConnection(client); if (!httpsSuccess && !mLatestResult.isSslRecoverableException()) { Log_OC.d(TAG, "establishing secure connection failed, trying non secure connection"); - client.setBaseUri(Uri.parse("http://" + baseUriStr)); + client.setBaseUri(Uri.parse(HTTP_PREFIX + baseUriStr)); tryConnection(client); } } diff --git a/src/com/owncloud/android/lib/resources/status/OwnCloudVersion.java b/src/com/owncloud/android/lib/resources/status/OwnCloudVersion.java index a2d830e7..2da2e564 100644 --- a/src/com/owncloud/android/lib/resources/status/OwnCloudVersion.java +++ b/src/com/owncloud/android/lib/resources/status/OwnCloudVersion.java @@ -48,6 +48,8 @@ public class OwnCloudVersion implements Comparable { private static final int MINIMUM_VERSION_WITH_SESSION_MONITORING_WORKING_IN_PREEMPTIVE_MODE = 0x09010301; // 9.1.3.1, final 9.1.3: https://github.com/owncloud/core/commit/f9a867b70c217463289a741d4d26079eb2a80dfd + private static final String INVALID_ZERO_VERSION = "0.0.0"; + private static final int MAX_DOTS = 3; // format is in version @@ -80,8 +82,11 @@ public class OwnCloudVersion implements Comparable { } public String toString() { + // gets the first digit of version, shifting hexadecimal version to right 'til max position String versionToString = String.valueOf((mVersion >> (8 * MAX_DOTS)) % 256); for (int i = MAX_DOTS - 1; i >= 0; i--) { + // gets another digit of version, shifting hexadecimal version to right 8*i bits and... + // ...discarding left part with mod 256 versionToString = versionToString + "." + String.valueOf((mVersion >> (8 * i)) % 256); } if (!mIsValid) { @@ -94,7 +99,7 @@ public class OwnCloudVersion implements Comparable { if (mIsValid) { return toString(); } else { - return "0.0.0"; + return INVALID_ZERO_VERSION; } } From 1f69348cc6c88ac56c0f46a7b0733d06b928ac6e Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Mon, 17 Apr 2017 13:49:00 +0200 Subject: [PATCH 3/3] Updated tests for OwnCloudClient after removing path constant ODAV_PATH --- .../android/lib/test_project/test/OwnCloudClientTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test_client/tests/src/com/owncloud/android/lib/test_project/test/OwnCloudClientTest.java b/test_client/tests/src/com/owncloud/android/lib/test_project/test/OwnCloudClientTest.java index fd4b556f..8bc64e3c 100644 --- a/test_client/tests/src/com/owncloud/android/lib/test_project/test/OwnCloudClientTest.java +++ b/test_client/tests/src/com/owncloud/android/lib/test_project/test/OwnCloudClientTest.java @@ -296,9 +296,8 @@ public class OwnCloudClientTest extends AndroidTestCase { new OwnCloudClient(mServerUri, NetworkUtils.getMultiThreadedConnManager()); client.setCredentials(OwnCloudCredentialsFactory.newBearerCredentials("fakeToken")); Uri webdavUri = client.getWebdavUri(); - assertTrue("WebDAV URI does not point to the right entry point for OAuth2 " + - "authenticated servers", - webdavUri.getPath().endsWith(AccountUtils.ODAV_PATH)); + assertTrue("WebDAV URI does not point to the right entry point", + webdavUri.getPath().endsWith(AccountUtils.WEBDAV_PATH_4_0)); assertTrue("WebDAV URI is not a subpath of base URI", webdavUri.getAuthority().equals(mServerUri.getAuthority()) && webdavUri.getPath().startsWith(mServerUri.getPath()));