diff --git a/dom/base/nsContentAreaDragDrop.cpp b/dom/base/nsContentAreaDragDrop.cpp index 771c27829f12..b03a3a8432a5 100644 --- a/dom/base/nsContentAreaDragDrop.cpp +++ b/dom/base/nsContentAreaDragDrop.cpp @@ -181,8 +181,7 @@ nsresult CheckAndGetExtensionForMime(const nsCString& aExtension, getter_AddRefs(mimeInfo)); NS_ENSURE_SUCCESS(rv, rv); - rv = mimeInfo->GetPrimaryExtension(*aPrimaryExtension); - NS_ENSURE_SUCCESS(rv, rv); + mimeInfo->GetPrimaryExtension(*aPrimaryExtension); if (aExtension.IsEmpty()) { *aIsValidExtension = false; @@ -279,7 +278,7 @@ nsContentAreaDragDropDataProvider::GetFlavorData(nsITransferable* aTransferable, &isValidExtension, &primaryExtension); NS_ENSURE_SUCCESS(rv, rv); - if (!isValidExtension) { + if (!isValidExtension && !primaryExtension.IsEmpty()) { // The filename extension is missing or incompatible // with the MIME type, replace it with the primary // extension. @@ -463,12 +462,13 @@ nsresult DragDataProducer::GetImageData(imgIContainer* aImage, // Fix the file extension in the URL nsAutoCString primaryExtension; mimeInfo->GetPrimaryExtension(primaryExtension); - - rv = NS_MutateURI(imgUrl) - .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension, - primaryExtension, nullptr)) - .Finalize(imgUrl); - NS_ENSURE_SUCCESS(rv, rv); + if (!primaryExtension.IsEmpty()) { + rv = NS_MutateURI(imgUrl) + .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension, + primaryExtension, nullptr)) + .Finalize(imgUrl); + NS_ENSURE_SUCCESS(rv, rv); + } } } #endif /* defined(XP_MACOSX) */ diff --git a/dom/base/nsCopySupport.cpp b/dom/base/nsCopySupport.cpp index f803a72bd410..73f62884738e 100644 --- a/dom/base/nsCopySupport.cpp +++ b/dom/base/nsCopySupport.cpp @@ -639,12 +639,13 @@ static nsresult AppendImagePromise(nsITransferable* aTransferable, // Fix the file extension in the URL nsAutoCString primaryExtension; mimeInfo->GetPrimaryExtension(primaryExtension); - - rv = NS_MutateURI(imgUri) - .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension, - primaryExtension, nullptr)) - .Finalize(imgUrl); - NS_ENSURE_SUCCESS(rv, rv); + if (!primaryExtension.IsEmpty()) { + rv = NS_MutateURI(imgUri) + .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension, + primaryExtension, nullptr)) + .Finalize(imgUrl); + NS_ENSURE_SUCCESS(rv, rv); + } } nsAutoCString fileName; diff --git a/dom/webbrowserpersist/nsWebBrowserPersist.cpp b/dom/webbrowserpersist/nsWebBrowserPersist.cpp index 9b884fd31f13..82c7a759efb0 100644 --- a/dom/webbrowserpersist/nsWebBrowserPersist.cpp +++ b/dom/webbrowserpersist/nsWebBrowserPersist.cpp @@ -1999,9 +1999,15 @@ nsresult nsWebBrowserPersist::CalculateAndAppendFileExt( mimeInfo->ExtensionExists(fileExt, &useOldExt); } - // can't use old extension so use primary extension + // If the url doesn't have an extension, or we don't know the extension, + // try to use the primary extension for the type. If we don't know the + // primary extension for the type, just continue with the url extension. if (!useOldExt) { - mimeInfo->GetPrimaryExtension(fileExt); + nsAutoCString primaryExt; + mimeInfo->GetPrimaryExtension(primaryExt); + if (!primaryExt.IsEmpty()) { + fileExt = primaryExt; + } } if (!fileExt.IsEmpty()) { diff --git a/uriloader/exthandler/HandlerServiceParent.cpp b/uriloader/exthandler/HandlerServiceParent.cpp index bdba857a9377..72f3a4be81e8 100644 --- a/uriloader/exthandler/HandlerServiceParent.cpp +++ b/uriloader/exthandler/HandlerServiceParent.cpp @@ -170,13 +170,18 @@ NS_IMETHODIMP ProxyMIMEInfo::SetFileExtensions(const nsACString& aExtensions) { /* boolean extensionExists (in AUTF8String aExtension); */ NS_IMETHODIMP ProxyMIMEInfo::ExtensionExists(const nsACString& aExtension, bool* _retval) { - *_retval = mProxyHandlerInfo->Extensions().Contains(aExtension); + *_retval = mProxyHandlerInfo->Extensions().Contains( + aExtension, nsCaseInsensitiveCStringArrayComparator()); return NS_OK; } /* void appendExtension (in AUTF8String aExtension); */ NS_IMETHODIMP ProxyMIMEInfo::AppendExtension(const nsACString& aExtension) { - mProxyHandlerInfo->Extensions().AppendElement(aExtension); + if (!aExtension.IsEmpty() && + !mProxyHandlerInfo->Extensions().Contains( + aExtension, nsCaseInsensitiveCStringArrayComparator())) { + mProxyHandlerInfo->Extensions().AppendElement(aExtension); + } return NS_OK; } @@ -185,6 +190,7 @@ NS_IMETHODIMP ProxyMIMEInfo::GetPrimaryExtension( nsACString& aPrimaryExtension) { const auto& extensions = mProxyHandlerInfo->Extensions(); if (extensions.IsEmpty()) { + aPrimaryExtension.Truncate(); return NS_ERROR_FAILURE; } aPrimaryExtension = extensions[0]; diff --git a/uriloader/exthandler/android/nsMIMEInfoAndroid.cpp b/uriloader/exthandler/android/nsMIMEInfoAndroid.cpp index 7978845c38db..35eeaa70d777 100644 --- a/uriloader/exthandler/android/nsMIMEInfoAndroid.cpp +++ b/uriloader/exthandler/android/nsMIMEInfoAndroid.cpp @@ -238,15 +238,16 @@ nsMIMEInfoAndroid::GetFileExtensions(nsIUTF8StringEnumerator** aResult) { NS_IMETHODIMP nsMIMEInfoAndroid::SetFileExtensions(const nsACString& aExtensions) { mExtensions.Clear(); - nsCString extList(aExtensions); - - int32_t breakLocation = -1; - while ((breakLocation = extList.FindChar(',')) != -1) { - mExtensions.AppendElement( - Substring(extList.get(), extList.get() + breakLocation)); - extList.Cut(0, breakLocation + 1); + nsACString::const_iterator start, end; + aExtensions.BeginReading(start); + aExtensions.EndReading(end); + while (start != end) { + nsACString::const_iterator cursor = start; + mozilla::Unused << FindCharInReadable(',', cursor, end); + AddUniqueExtension(Substring(start, cursor)); + // If a comma was found, skip it for the next search. + start = cursor != end ? ++cursor : cursor; } - if (!extList.IsEmpty()) mExtensions.AppendElement(extList); return NS_OK; } @@ -268,38 +269,43 @@ nsMIMEInfoAndroid::ExtensionExists(const nsACString& aExtension, return NS_OK; } +void nsMIMEInfoAndroid::AddUniqueExtension(const nsACString& aExtension) { + if (!aExtension.IsEmpty() && + !mExtensions.Contains(aExtension, + nsCaseInsensitiveCStringArrayComparator())) { + mExtensions.AppendElement(aExtension); + } +} + NS_IMETHODIMP nsMIMEInfoAndroid::AppendExtension(const nsACString& aExtension) { - mExtensions.AppendElement(aExtension); + MOZ_ASSERT(!aExtension.IsEmpty(), "No extension"); + AddUniqueExtension(aExtension); return NS_OK; } NS_IMETHODIMP nsMIMEInfoAndroid::GetPrimaryExtension(nsACString& aPrimaryExtension) { - if (!mExtensions.Length()) return NS_ERROR_NOT_INITIALIZED; - + if (!mExtensions.Length()) { + aPrimaryExtension.Truncate(); + return NS_ERROR_NOT_INITIALIZED; + } aPrimaryExtension = mExtensions[0]; return NS_OK; } NS_IMETHODIMP nsMIMEInfoAndroid::SetPrimaryExtension(const nsACString& aExtension) { - uint32_t extCount = mExtensions.Length(); - uint8_t i; - bool found = false; - for (i = 0; i < extCount; i++) { - const nsCString& ext = mExtensions[i]; - if (ext.Equals(aExtension, nsCaseInsensitiveCStringComparator())) { - found = true; - break; - } + if (MOZ_UNLIKELY(aExtension.IsEmpty())) { + // Don't assert since Java may return an empty extension for unknown types. + return NS_ERROR_INVALID_ARG; } - if (found) { + int32_t i = mExtensions.IndexOf(aExtension, 0, + nsCaseInsensitiveCStringArrayComparator()); + if (i != -1) { mExtensions.RemoveElementAt(i); } - mExtensions.InsertElementAt(0, aExtension); - return NS_OK; } diff --git a/uriloader/exthandler/android/nsMIMEInfoAndroid.h b/uriloader/exthandler/android/nsMIMEInfoAndroid.h index 038f70e0ba6e..5a0544156832 100644 --- a/uriloader/exthandler/android/nsMIMEInfoAndroid.h +++ b/uriloader/exthandler/android/nsMIMEInfoAndroid.h @@ -30,6 +30,11 @@ class nsMIMEInfoAndroid final : public nsIMIMEInfo { private: ~nsMIMEInfoAndroid() {} + /** + * Internal helper to avoid adding duplicates. + */ + void AddUniqueExtension(const nsACString& aExtension); + virtual MOZ_MUST_USE nsresult LaunchDefaultWithFile(nsIFile* aFile); virtual MOZ_MUST_USE nsresult LoadUriInternal(nsIURI* aURI); nsCOMPtr mHandlerApps; diff --git a/uriloader/exthandler/nsMIMEInfoImpl.cpp b/uriloader/exthandler/nsMIMEInfoImpl.cpp index 30ee38e8cf3a..91ad5d1c1509 100644 --- a/uriloader/exthandler/nsMIMEInfoImpl.cpp +++ b/uriloader/exthandler/nsMIMEInfoImpl.cpp @@ -95,56 +95,49 @@ nsMIMEInfoBase::GetFileExtensions(nsIUTF8StringEnumerator** aResult) { NS_IMETHODIMP nsMIMEInfoBase::ExtensionExists(const nsACString& aExtension, bool* _retval) { - NS_ASSERTION(!aExtension.IsEmpty(), "no extension"); - bool found = false; - uint32_t extCount = mExtensions.Length(); - if (extCount < 1) return NS_OK; - - for (uint8_t i = 0; i < extCount; i++) { - const nsCString& ext = mExtensions[i]; - if (ext.Equals(aExtension, nsCaseInsensitiveCStringComparator())) { - found = true; - break; - } - } - - *_retval = found; + MOZ_ASSERT(!aExtension.IsEmpty(), "no extension"); + *_retval = mExtensions.Contains(aExtension, + nsCaseInsensitiveCStringArrayComparator()); return NS_OK; } NS_IMETHODIMP nsMIMEInfoBase::GetPrimaryExtension(nsACString& _retval) { - if (!mExtensions.Length()) return NS_ERROR_NOT_INITIALIZED; - + if (!mExtensions.Length()) { + _retval.Truncate(); + return NS_ERROR_NOT_INITIALIZED; + } _retval = mExtensions[0]; return NS_OK; } NS_IMETHODIMP nsMIMEInfoBase::SetPrimaryExtension(const nsACString& aExtension) { - NS_ASSERTION(!aExtension.IsEmpty(), "no extension"); - uint32_t extCount = mExtensions.Length(); - uint8_t i; - bool found = false; - for (i = 0; i < extCount; i++) { - const nsCString& ext = mExtensions[i]; - if (ext.Equals(aExtension, nsCaseInsensitiveCStringComparator())) { - found = true; - break; - } + if (MOZ_UNLIKELY(aExtension.IsEmpty())) { + MOZ_ASSERT(false, "No extension"); + return NS_ERROR_INVALID_ARG; } - if (found) { + int32_t i = mExtensions.IndexOf(aExtension, 0, + nsCaseInsensitiveCStringArrayComparator()); + if (i != -1) { mExtensions.RemoveElementAt(i); } - mExtensions.InsertElementAt(0, aExtension); - return NS_OK; } +void nsMIMEInfoBase::AddUniqueExtension(const nsACString& aExtension) { + if (!aExtension.IsEmpty() && + !mExtensions.Contains(aExtension, + nsCaseInsensitiveCStringArrayComparator())) { + mExtensions.AppendElement(aExtension); + } +} + NS_IMETHODIMP nsMIMEInfoBase::AppendExtension(const nsACString& aExtension) { - mExtensions.AppendElement(aExtension); + MOZ_ASSERT(!aExtension.IsEmpty(), "No extension"); + AddUniqueExtension(aExtension); return NS_OK; } @@ -192,15 +185,16 @@ nsMIMEInfoBase::Equals(nsIMIMEInfo* aMIMEInfo, bool* _retval) { NS_IMETHODIMP nsMIMEInfoBase::SetFileExtensions(const nsACString& aExtensions) { mExtensions.Clear(); - nsCString extList(aExtensions); - - int32_t breakLocation = -1; - while ((breakLocation = extList.FindChar(',')) != -1) { - mExtensions.AppendElement( - Substring(extList.get(), extList.get() + breakLocation)); - extList.Cut(0, breakLocation + 1); + nsACString::const_iterator start, end; + aExtensions.BeginReading(start); + aExtensions.EndReading(end); + while (start != end) { + nsACString::const_iterator cursor = start; + mozilla::Unused << FindCharInReadable(',', cursor, end); + AddUniqueExtension(Substring(start, cursor)); + // If a comma was found, skip it for the next search. + start = cursor != end ? ++cursor : cursor; } - if (!extList.IsEmpty()) mExtensions.AppendElement(extList); return NS_OK; } diff --git a/uriloader/exthandler/nsMIMEInfoImpl.h b/uriloader/exthandler/nsMIMEInfoImpl.h index 6beffc20a0b7..255fdc9aeaee 100644 --- a/uriloader/exthandler/nsMIMEInfoImpl.h +++ b/uriloader/exthandler/nsMIMEInfoImpl.h @@ -137,6 +137,11 @@ class nsMIMEInfoBase : public nsIMIMEInfo { */ static nsresult GetLocalFileFromURI(nsIURI* aURI, nsIFile** aFile); + /** + * Internal helper to avoid adding duplicates. + */ + void AddUniqueExtension(const nsACString& aExtension); + // member variables nsTArray mExtensions; ///< array of file extensions associated w/ this MIME obj diff --git a/uriloader/exthandler/tests/unit/test_handlerService_store.js b/uriloader/exthandler/tests/unit/test_handlerService_store.js index a543da40142c..aa2efde822f3 100644 --- a/uriloader/exthandler/tests/unit/test_handlerService_store.js +++ b/uriloader/exthandler/tests/unit/test_handlerService_store.js @@ -53,8 +53,7 @@ function assertAllHandlerInfosMatchTestData() { // It's important that the MIME types we check here do not exist at the // operating system level, otherwise the list of handlers and file extensions - // will be merged. The current implementation adds each saved file extension - // even if one already exists in the system, resulting in duplicate entries. + // will be merged. The current implementation avoids duplicate entries. HandlerServiceTestUtils.assertHandlerInfoMatches(handlerInfos.shift(), { type: "example/type.handleinternally", @@ -415,8 +414,8 @@ add_task(async function test_store_fileExtensions_lowercase() { }); /** - * Tests that duplicates added with "appendExtension" or present in - * "possibleApplicationHandlers" are removed when saving and reloading. + * Tests that appendExtension doesn't add duplicates, and that anyway duplicates + * from possibleApplicationHandlers are removed when saving and reloading. */ add_task(async function test_store_no_duplicates() { await deleteHandlerStore(); @@ -431,6 +430,10 @@ add_task(async function test_store_no_duplicates() { handlerInfo.appendExtension("extension_test2"); handlerInfo.appendExtension("extension_test1"); handlerInfo.appendExtension("EXTENSION_test1"); + Assert.deepEqual(Array.from(handlerInfo.getFileExtensions()), [ + "extension_test1", + "extension_test2", + ]); gHandlerService.store(handlerInfo); await unloadHandlerStore(); @@ -449,6 +452,21 @@ add_task(async function test_store_no_duplicates() { }); }); +/** + * Tests that setFileExtensions doesn't add duplicates. + */ +add_task(async function test_setFileExtensions_no_duplicates() { + await deleteHandlerStore(); + + let handlerInfo = getKnownHandlerInfo("example/new"); + handlerInfo.setFileExtensions("a,b,A,b,c,a"); + let expected = ["a", "b", "c"]; + Assert.deepEqual(Array.from(handlerInfo.getFileExtensions()), expected); + // Test empty extensions, also at begin and end. + handlerInfo.setFileExtensions(",a,,b,A,c,"); + Assert.deepEqual(Array.from(handlerInfo.getFileExtensions()), expected); +}); + /** * Tests that "store" deletes properties that have their default values from * the data store.