Commit e33f7976 authored by Pete Williamson's avatar Pete Williamson Committed by Commit Bot

Add unit tests for page sharing, and check for sharing from private dir.

We should not be sharing files from our private directory, they are not
sharable, and could cause a crash.  This change marks the file as not
sharable (when the P2P sharing flag is turned on) so we avoid the crash.

To tell if the file is in a private directory, we add a new file to the
OfflinePageBridge to get the information from the C++ side.

Previously there were no unit tests for maybeShareOfflinePage().  This
change adds some unit tests for it as a basis for checking the sharing
restriction, then adds more tests for the sharing restriction.


Bug: 817611
Change-Id: Ic0361a6eaf2698562a4998c2e027081c76318bc5
Reviewed-on: https://chromium-review.googlesource.com/961542
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: default avatarJian Li <jianli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543851}
parent 9c4d192f
......@@ -646,6 +646,15 @@ public class OfflinePageBridge {
return nativeIsOfflinePage(mNativeOfflinePageBridge, webContents);
}
/**
* Checks if the supplied file path is in a private dir internal to chrome.
* @param file_path Path of the file to check.
* @return True if the file is in a private directory.
*/
public boolean isInPrivateDirectory(String filePath) {
return nativeIsInPrivateDirectory(mNativeOfflinePageBridge, filePath);
}
/**
* Retrieves the offline page that is shown for the tab.
* @param webContents Web contents used to find the offline page.
......@@ -820,6 +829,8 @@ public class OfflinePageBridge {
WebContents webContents, String nameSpace, String url, int uiAction, String origin);
private native boolean nativeIsOfflinePage(
long nativeOfflinePageBridge, WebContents webContents);
private native boolean nativeIsInPrivateDirectory(
long nativeOfflinePageBridge, String filePath);
private native OfflinePageItem nativeGetOfflinePage(
long nativeOfflinePageBridge, WebContents webContents);
private native void nativeCheckForNewOfflineContent(
......
......@@ -367,17 +367,9 @@ public class OfflinePageUtils {
}
OfflinePageItem offlinePage = offlinePageBridge.getOfflinePage(tab.getWebContents());
// Bail if there is no offline page or sharing is not enabled.
if (offlinePage == null || !OfflinePageBridge.isPageSharingEnabled()) return false;
// If we share a page with a content URI, it will not have a file path.
// We cannot share it without a file path, so return false. That will give other
// sharing methods a chance to run.
String offlinePath = offlinePage.getFilePath();
if (offlinePath.isEmpty()) {
Log.w(TAG, "Tried to share a page with no path.");
return false;
}
if (!isOfflinePageShareable(offlinePageBridge, offlinePage)) return false;
final String tabTitle = tab.getTitle();
final String tabUrl = tab.getUrl();
......@@ -401,6 +393,31 @@ public class OfflinePageUtils {
return true;
}
/**
* Check to see if the offline page is sharable.
* @param offlinePage Page to check for sharability.
* @return true if this page can be shared.
*/
public static boolean isOfflinePageShareable(
OfflinePageBridge offlinePageBridge, OfflinePageItem offlinePage) {
// Return false if there is no offline page or sharing is not enabled.
if (offlinePage == null || !OfflinePageBridge.isPageSharingEnabled()) return false;
// We cannot share a file without a file path, so return false.
// TODO(petewil) Allow sharing if there is a content or file URI, even if there is no path.
// https://crbug.com/817608
String offlinePath = offlinePage.getFilePath();
if (offlinePath.isEmpty()) {
Log.w(TAG, "Tried to share a page with no path.");
return false;
}
// If the page is not in a public location, we cannot share it.
if (offlinePageBridge.isInPrivateDirectory(offlinePath)) return false;
return true;
}
/**
* Retrieves the extra request header to reload the offline page.
* @param tab The current tab.
......
......@@ -790,6 +790,14 @@ jboolean OfflinePageBridge::IsOfflinePage(
web_contents) != nullptr;
}
jboolean OfflinePageBridge::IsInPrivateDirectory(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jstring>& j_file_path) {
base::FilePath file_path(ConvertJavaStringToUTF8(env, j_file_path));
return offline_page_model_->IsArchiveInInternalDir(file_path);
}
ScopedJavaLocalRef<jobject> OfflinePageBridge::GetOfflinePage(
JNIEnv* env,
const JavaParamRef<jobject>& obj,
......
......@@ -179,6 +179,11 @@ class OfflinePageBridge : public OfflinePageModel::Observer,
const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jobject>& j_web_contents);
jboolean IsInPrivateDirectory(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jstring>& j_file_path);
base::android::ScopedJavaLocalRef<jobject> GetOfflinePage(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment