Commit 1c96fe78 authored by Takashi Sakamoto's avatar Takashi Sakamoto Committed by Commit Bot

Revert "Check file mime type provided by ARC before appending a file extension."

This reverts commit 1e471d39.

Reason for revert:
ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension is failling on Linux Chromium OS Asan Lsan Tests:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/28834
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/28835

[ RUN      ] ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:273: Failure
Expected equality of these values:
  "application/msword"
    Which is: 0x2de8e960
  FindArcMimeTypeFromExtension("doc")
    Which is: 0x2eaa2520
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:276: Failure
Expected equality of these values:
  "video/3gpp"
    Which is: 0x2de8e520
  FindArcMimeTypeFromExtension("3gp")
    Which is: 0x2eaa3aa0
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:279: Failure
Expected equality of these values:
  "audio/mpeg"
    Which is: 0x2de8eb60
  FindArcMimeTypeFromExtension("mpga")
    Which is: 0x2eaa2d60
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
[  FAILED  ] ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension (1 ms)
[ RUN      ] ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:273: Failure
Expected equality of these values:
  "application/msword"
    Which is: 0x2de8e960
  FindArcMimeTypeFromExtension("doc")
    Which is: 0x2eaa2520
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:276: Failure
Expected equality of these values:
  "video/3gpp"
    Which is: 0x2de8e520
  FindArcMimeTypeFromExtension("3gp")
    Which is: 0x2eaa3aa0
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:279: Failure
Expected equality of these values:
  "audio/mpeg"
    Which is: 0x2de8eb60
  FindArcMimeTypeFromExtension("mpga")
    Which is: 0x2eaa2d60
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
[  FAILED  ] ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension (2 ms)
[ RUN      ] ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:273: Failure
Expected equality of these values:
  "application/msword"
    Which is: 0x2de8e960
  FindArcMimeTypeFromExtension("doc")
    Which is: 0x2eaa2520
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:276: Failure
Expected equality of these values:
  "video/3gpp"
    Which is: 0x2de8e520
  FindArcMimeTypeFromExtension("3gp")
    Which is: 0x2eaa3aa0
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:279: Failure
Expected equality of these values:
  "audio/mpeg"
    Which is: 0x2de8eb60
  FindArcMimeTypeFromExtension("mpga")
    Which is: 0x2eaa2d60
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
[  FAILED  ] ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension (1 ms)
[ RUN      ] ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:273: Failure
Expected equality of these values:
  "application/msword"
    Which is: 0x2de8e960
  FindArcMimeTypeFromExtension("doc")
    Which is: 0x2eaa2520
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:276: Failure
Expected equality of these values:
  "video/3gpp"
    Which is: 0x2de8e520
  FindArcMimeTypeFromExtension("3gp")
    Which is: 0x2eaa3aa0
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
../../chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util_unittest.cc:279: Failure
Expected equality of these values:
  "audio/mpeg"
    Which is: 0x2de8eb60
  FindArcMimeTypeFromExtension("mpga")
    Which is: 0x2eaa2d60
Stack trace:
#0 0x000008bf16ff (/b/s/w/ir/out/Release/unit_tests+0x8bf16fe)
#1 0x000008c17af0 (/b/s/w/ir/out/Release/unit_tests+0x8c17aef)
#2 0x000008c166ca (/b/s/w/ir/out/Release/unit_tests+0x8c166c9)
[  FAILED  ] ArcDocumentsProviderUtilTest.FindArcMimeTypeFromExtension (2 ms)

Original change's description:
> Check file mime type provided by ARC before appending a file extension.
> 
> Some files are returned with an unexpected mime type (eg. 3gp and video/mp4) causing an extra
> extension to be appended when it is not in the list of allowed extensions returned by
> net::GetExtensionsForMimeType. In those cases, do an extra lookup in the Android mapping by
> extension, and don't append the extension if a valid MIME type is returned and the MIME
> category type (ie. MIME type without the subtype) matches.
> 
> Bug: 844252
> Test: Tested on kevin.
> Change-Id: I9899d74d6046d6c392adc4f828af09296c73f1c6
> Reviewed-on: https://chromium-review.googlesource.com/1189690
> Commit-Queue: Alex Lau <alexlau@chromium.org>
> Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
> Reviewed-by: Shuhei Takahashi <nya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#588444}

TBR=hidehiko@chromium.org,nya@chromium.org,alexlau@chromium.org

Change-Id: I6b77806153d942e9f996624a2d782448371c04bb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 844252
Reviewed-on: https://chromium-review.googlesource.com/1203353Reviewed-by: default avatarTakashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#588463}
parent 7d117ead
...@@ -217,30 +217,6 @@ std::vector<base::FilePath::StringType> GetExtensionsForArcMimeType( ...@@ -217,30 +217,6 @@ std::vector<base::FilePath::StringType> GetExtensionsForArcMimeType(
return std::vector<base::FilePath::StringType>(); return std::vector<base::FilePath::StringType>();
} }
std::string StripMimeSubType(const std::string& mime_type) {
if (mime_type.empty())
return mime_type;
size_t index = mime_type.find_first_of('/', 0);
if (index == 0 || index == mime_type.size() - 1 ||
index == std::string::npos) {
// This looks malformed, return an empty string.
return std::string();
}
return mime_type.substr(0, index);
}
// This is based on net/base/mime_util.cc: net::FindMimeType.
const char* FindArcMimeTypeFromExtension(const std::string& ext) {
for (const auto& mapping : kAndroidMimeTypeMappings) {
std::vector<base::StringPiece> extensions = base::SplitStringPiece(
mapping.extensions, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
auto iter = std::find(extensions.begin(), extensions.end(), ext);
if (iter != extensions.end())
return mapping.mime_type;
}
return nullptr;
}
// TODO(crbug.com/675868): Consolidate with the similar logic for Drive. // TODO(crbug.com/675868): Consolidate with the similar logic for Drive.
base::FilePath::StringType GetFileNameForDocument( base::FilePath::StringType GetFileNameForDocument(
const mojom::DocumentPtr& document) { const mojom::DocumentPtr& document) {
...@@ -267,23 +243,10 @@ base::FilePath::StringType GetFileNameForDocument( ...@@ -267,23 +243,10 @@ base::FilePath::StringType GetFileNameForDocument(
extension = extension.substr(1); // Strip the leading dot. extension = extension.substr(1); // Strip the leading dot.
std::vector<base::FilePath::StringType> possible_extensions = std::vector<base::FilePath::StringType> possible_extensions =
GetExtensionsForArcMimeType(document->mime_type); GetExtensionsForArcMimeType(document->mime_type);
if (!possible_extensions.empty() && if (!possible_extensions.empty() &&
!base::ContainsValue(possible_extensions, extension)) { !base::ContainsValue(possible_extensions, extension)) {
// Lookup the extension in the hardcoded map before appending an extension, filename =
// as some extensions (eg. 3gp) are typed differently by Android. Only base::FilePath(filename).AddExtension(possible_extensions[0]).value();
// append the suggested extension if the lookup fails (i.e. no valid mime
// type returned), or the returned mime type is of a different category.
// TODO(crbug.com/878221): Fix discrepancy in MIME types and extensions
// between the hard coded map and the Android content provider.
const char* missed_possible_mime_type =
FindArcMimeTypeFromExtension(extension);
if (missed_possible_mime_type == nullptr ||
StripMimeSubType(document->mime_type) !=
StripMimeSubType(missed_possible_mime_type)) {
filename =
base::FilePath(filename).AddExtension(possible_extensions[0]).value();
}
} }
return filename; return filename;
......
...@@ -75,14 +75,6 @@ std::vector<base::FilePath::StringType> GetExtensionsForArcMimeType( ...@@ -75,14 +75,6 @@ std::vector<base::FilePath::StringType> GetExtensionsForArcMimeType(
base::FilePath::StringType GetFileNameForDocument( base::FilePath::StringType GetFileNameForDocument(
const mojom::DocumentPtr& document); const mojom::DocumentPtr& document);
// Returns the provided MIME type without the subtype component.
std::string StripMimeSubType(const std::string& mime_type);
// Finds the first matching mime type with |ext| as a valid extension from the
// internal list of Android mime types. On success, the first matching MIME type
// is returned. On failure, nullptr is returned.
const char* FindArcMimeTypeFromExtension(const std::string& ext);
} // namespace arc } // namespace arc
#endif // CHROME_BROWSER_CHROMEOS_ARC_FILEAPI_ARC_DOCUMENTS_PROVIDER_UTIL_H_ #endif // CHROME_BROWSER_CHROMEOS_ARC_FILEAPI_ARC_DOCUMENTS_PROVIDER_UTIL_H_
...@@ -243,49 +243,6 @@ TEST(ArcDocumentsProviderUtilTest, GetFileNameForDocument) { ...@@ -243,49 +243,6 @@ TEST(ArcDocumentsProviderUtilTest, GetFileNameForDocument) {
GetFileNameForDocument(MakeDocument("kitten", "image/png"))); GetFileNameForDocument(MakeDocument("kitten", "image/png")));
EXPECT_EQ("kitten", EXPECT_EQ("kitten",
GetFileNameForDocument(MakeDocument("kitten", "abc/xyz"))); GetFileNameForDocument(MakeDocument("kitten", "abc/xyz")));
// Check that files with a mime type different than expected appends a
// possible extension when a different mime type with a different category is
// found in the Android mime types map.
EXPECT_EQ("file.txt.3gp",
GetFileNameForDocument(MakeDocument("file.txt", "video/3gpp")));
// Check that files with a mime type different than expected don't append
// an extension when a different mime type with the same category is
// found in the Android mime types map.
EXPECT_EQ("file.3gp",
GetFileNameForDocument(MakeDocument("file.3gp", "video/mp4")));
}
TEST(ArcDocumentsProviderUtilTest, StripMimeSubType) {
// Check that the category type is returned for a valid mime type.
EXPECT_EQ("video", StripMimeSubType("video/mp4"));
// Check that an empty string is returned for malformed mime types.
EXPECT_EQ("", StripMimeSubType(""));
EXPECT_EQ("", StripMimeSubType("video/"));
EXPECT_EQ("", StripMimeSubType("/"));
EXPECT_EQ("", StripMimeSubType("/abc"));
EXPECT_EQ("", StripMimeSubType("/abc/xyz"));
}
TEST(ArcDocumentsProviderUtilTest, FindArcMimeTypeFromExtension) {
// Test that a lone possible extension returns the correct type.
EXPECT_EQ("application/msword", FindArcMimeTypeFromExtension("doc"));
// Test that the first extension in the comma delimited list of extensions
// returns the correct type.
EXPECT_EQ("video/3gpp", FindArcMimeTypeFromExtension("3gp"));
// Test that the second extension in the comma delimited list of extensions
// returns the correct type.
EXPECT_EQ("audio/mpeg", FindArcMimeTypeFromExtension("mpga"));
// Test that matching suffixes (m4a) return null without a full match.
EXPECT_EQ(nullptr, FindArcMimeTypeFromExtension("4a"));
// Test that matching prefixes (imy) return null without a full match.
EXPECT_EQ(nullptr, FindArcMimeTypeFromExtension("im"));
// Test that ambiguous prefixes (mp4, mpg) return null.
EXPECT_EQ(nullptr, FindArcMimeTypeFromExtension("mp"));
// Test that invalid mime types return null.
EXPECT_EQ(nullptr, FindArcMimeTypeFromExtension(""));
EXPECT_EQ(nullptr, FindArcMimeTypeFromExtension("invalid"));
} }
} // namespace } // namespace
......
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