richard pushed to branch tor-browser-115.11.0esr-13.0-1 at The Tor Project / Applications / Tor Browser

Commits: 51f80eb6 by Kershaw Chang at 2024-05-10T22:22:15+02:00 Bug 1870579 - Use PK11_GenerateRandom to generate random number, r=necko-reviewers,valentin

Differential Revision: https://phabricator.services.mozilla.com/D204755

- - - - - cb7f8ec0 by Kershaw Chang at 2024-05-10T22:22:56+02:00 Bug 1892449 - Set network.http.digest_auth_cnonce_length to 16, a=dmeehan

Apparently, setting this value to 64 breaks some sites. We should use the same length as Chrome.

Original Revision: https://phabricator.services.mozilla.com/D208103

Differential Revision: https://phabricator.services.mozilla.com/D208119 - - - - - 2e33d3b5 by Nika Layzell at 2024-05-10T22:37:57+02:00 Bug 1875248 - Check for network error preventing ExternalHelperAppService before DONT_RETARGET, r=smaug

This reverts the change from 30cde47f9364e5c7da78fd08fa8ab21737d22399, and instead re-orders the NS_ERROR_FILE_NOT_FOUND check before DONT_RETARGET.

Testing suggests that a-download-click-404.html behaviour isn't impacted, and this improves the handling of this edge-case when doing process switching.

Differential Revision: https://phabricator.services.mozilla.com/D202007 - - - - - bbb3af71 by Jonathan Kew at 2024-05-10T23:19:01+02:00 Bug 1886598 - Struct with Pointer member may not be memmove-able. r=gfx-reviewers,lsalzman

Differential Revision: https://phabricator.services.mozilla.com/D206633 - - - - - f87ae3f1 by alwu at 2024-05-10T23:34:34+02:00 Bug 1644383 - add mutexs to avoid data race. r=media-playback-reviewers,padenot

Differential Revision: https://phabricator.services.mozilla.com/D206943 - - - - -

8 changed files:

- dom/media/ipc/RemoteMediaDataDecoder.cpp - dom/media/ipc/RemoteMediaDataDecoder.h - dom/media/platforms/wrappers/MediaChangeMonitor.cpp - dom/media/platforms/wrappers/MediaChangeMonitor.h - gfx/thebes/gfxPlatformFontList.h - modules/libpref/init/StaticPrefList.yaml - netwerk/protocol/http/nsHttpDigestAuth.cpp - uriloader/base/nsURILoader.cpp

Changes:

===================================== dom/media/ipc/RemoteMediaDataDecoder.cpp ===================================== @@ -18,7 +18,12 @@ namespace mozilla { ##__VA_ARGS__)

RemoteMediaDataDecoder::RemoteMediaDataDecoder(RemoteDecoderChild* aChild) - : mChild(aChild) { + : mChild(aChild), + mDescription("RemoteMediaDataDecoder"_ns), + mProcessName("unknown"_ns), + mCodecName("unknown"_ns), + mIsHardwareAccelerated(false), + mConversion(ConversionRequired::kNeedNone) { LOG("%p is created", this); }

@@ -48,6 +53,7 @@ RefPtrMediaDataDecoder::InitPromise RemoteMediaDataDecoder::Init() { ->Then( RemoteDecoderManagerChild::GetManagerThread(), __func__, [self, this](TrackType aTrack) { + MutexAutoLock lock(mMutex); // If shutdown has started in the meantime shutdown promise may // be resloved before this task. In this case mChild will be null // and the init promise has to be canceled. @@ -127,6 +133,7 @@ RefPtr<ShutdownPromise> RemoteMediaDataDecoder::Shutdown() {

bool RemoteMediaDataDecoder::IsHardwareAccelerated( nsACString& aFailureReason) const { + MutexAutoLock lock(mMutex); aFailureReason = mHardwareAcceleratedReason; return mIsHardwareAccelerated; } @@ -145,18 +152,24 @@ void RemoteMediaDataDecoder::SetSeekThreshold(const media::TimeUnit& aTime) {

MediaDataDecoder::ConversionRequired RemoteMediaDataDecoder::NeedsConversion() const { + MutexAutoLock lock(mMutex); return mConversion; }

nsCString RemoteMediaDataDecoder::GetDescriptionName() const { + MutexAutoLock lock(mMutex); return mDescription; }

nsCString RemoteMediaDataDecoder::GetProcessName() const { + MutexAutoLock lock(mMutex); return mProcessName; }

-nsCString RemoteMediaDataDecoder::GetCodecName() const { return mCodecName; } +nsCString RemoteMediaDataDecoder::GetCodecName() const { + MutexAutoLock lock(mMutex); + return mCodecName; +}

#undef LOG

===================================== dom/media/ipc/RemoteMediaDataDecoder.h ===================================== @@ -53,14 +53,16 @@ class RemoteMediaDataDecoder final // destructor when we can guarantee no other threads are accessing it). Only // read from the manager thread. RefPtr<RemoteDecoderChild> mChild; + + mutable Mutex mMutex{"RemoteMediaDataDecoder"}; + // Only ever written/modified during decoder initialisation. - // As such can be accessed from any threads after that. - nsCString mDescription = "RemoteMediaDataDecoder"_ns; - nsCString mProcessName = "unknown"_ns; - nsCString mCodecName = "unknown"_ns; - bool mIsHardwareAccelerated = false; - nsCString mHardwareAcceleratedReason; - ConversionRequired mConversion = ConversionRequired::kNeedNone; + nsCString mDescription MOZ_GUARDED_BY(mMutex); + nsCString mProcessName MOZ_GUARDED_BY(mMutex); + nsCString mCodecName MOZ_GUARDED_BY(mMutex); + bool mIsHardwareAccelerated MOZ_GUARDED_BY(mMutex); + nsCString mHardwareAcceleratedReason MOZ_GUARDED_BY(mMutex); + ConversionRequired mConversion MOZ_GUARDED_BY(mMutex); };

} // namespace mozilla

===================================== dom/media/platforms/wrappers/MediaChangeMonitor.cpp ===================================== @@ -668,6 +668,7 @@ RefPtr<ShutdownPromise> MediaChangeMonitor::ShutdownDecoder() { AssertOnThread(); mConversionRequired.reset(); if (mDecoder) { + MutexAutoLock lock(mMutex); RefPtr<MediaDataDecoder> decoder = std::move(mDecoder); return decoder->Shutdown(); } @@ -715,6 +716,7 @@ MediaChangeMonitor::CreateDecoder() { ->Then( GetCurrentSerialEventTarget(), __func__, [self = RefPtr{this}, this](RefPtr<MediaDataDecoder>&& aDecoder) { + MutexAutoLock lock(mMutex); mDecoder = std::move(aDecoder); DDLINKCHILD("decoder", mDecoder.get()); return CreateDecoderPromise::CreateAndResolve(true, __func__); @@ -963,6 +965,11 @@ void MediaChangeMonitor::FlushThenShutdownDecoder( ->Track(mFlushRequest); }

+MediaDataDecoder* MediaChangeMonitor::GetDecoderOnNonOwnerThread() const { + MutexAutoLock lock(mMutex); + return mDecoder; +} + #undef LOG

} // namespace mozilla

===================================== dom/media/platforms/wrappers/MediaChangeMonitor.h ===================================== @@ -41,34 +41,34 @@ class MediaChangeMonitor final RefPtr<ShutdownPromise> Shutdown() override; bool IsHardwareAccelerated(nsACString& aFailureReason) const override; nsCString GetDescriptionName() const override { - if (mDecoder) { - return mDecoder->GetDescriptionName(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->GetDescriptionName(); } return "MediaChangeMonitor decoder (pending)"_ns; } nsCString GetProcessName() const override { - if (mDecoder) { - return mDecoder->GetProcessName(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->GetProcessName(); } return "MediaChangeMonitor"_ns; } nsCString GetCodecName() const override { - if (mDecoder) { - return mDecoder->GetCodecName(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->GetCodecName(); } return "MediaChangeMonitor"_ns; } void SetSeekThreshold(const media::TimeUnit& aTime) override; bool SupportDecoderRecycling() const override { - if (mDecoder) { - return mDecoder->SupportDecoderRecycling(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->SupportDecoderRecycling(); } return false; }

ConversionRequired NeedsConversion() const override { - if (mDecoder) { - return mDecoder->NeedsConversion(); + if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) { + return decoder->NeedsConversion(); } // Default so no conversion is performed. return ConversionRequired::kNeedNone; @@ -97,6 +97,9 @@ class MediaChangeMonitor final MOZ_ASSERT(!mThread || mThread->IsOnCurrentThread()); }

+ // This is used for getting decoder debug info on other threads. Thread-safe. + MediaDataDecoder* GetDecoderOnNonOwnerThread() const; + bool CanRecycleDecoder() const;

typedef MozPromise<bool, MediaResult, true /* exclusive */> @@ -137,6 +140,13 @@ class MediaChangeMonitor final const CreateDecoderParamsForAsync mParams; // Keep any seek threshold set for after decoder creation and initialization. Maybemedia::TimeUnit mPendingSeekThreshold; + + // This lock is used for mDecoder specifically, but it doens't need to be used + // for every places accessing mDecoder which is mostly on the owner thread. + // However, when requesting decoder debug info, it can happen on other + // threads, so we need this mutex to avoid the data race of + // creating/destroying decoder and accessing decoder's debug info. + mutable Mutex MOZ_ANNOTATED mMutex{"MediaChangeMonitor"}; };

} // namespace mozilla

===================================== gfx/thebes/gfxPlatformFontList.h ===================================== @@ -124,7 +124,7 @@ class ShmemCharMapHashEntry final : public PLDHashEntryHdr { return aCharMap->GetChecksum(); }

- enum { ALLOW_MEMMOVE = true }; + enum { ALLOW_MEMMOVE = false }; // because of the Pointer member

private: // charMaps are stored in the shared memory that FontList objects point to,

===================================== modules/libpref/init/StaticPrefList.yaml ===================================== @@ -12787,6 +12787,18 @@ value: true mirror: always

+ # The length of cnonce string used in HTTP digest auth. +- name: network.http.digest_auth_cnonce_length + type: uint32_t + value: 16 + mirror: always + + # If true, HTTP response content-type headers will be parsed using the standards-compliant MimeType parser +- name: network.standard_content_type_parsing.response_headers + type: RelaxedAtomicBool + value: true + mirror: always + # The maximum count that we allow socket prrocess to crash. If this count is # reached, we won't use networking over socket process. - name: network.max_socket_process_failed_count

===================================== netwerk/protocol/http/nsHttpDigestAuth.cpp ===================================== @@ -9,6 +9,7 @@

#include "mozilla/ClearOnShutdown.h" #include "mozilla/Sprintf.h" +#include "mozilla/StaticPrefs_network.h" #include "mozilla/Unused.h"

#include "nsHttp.h" @@ -22,6 +23,7 @@ #include "nsCRT.h" #include "nsICryptoHash.h" #include "nsComponentManagerUtils.h" +#include "pk11pub.h"

constexpr uint16_t DigestLength(uint16_t aAlgorithm) { if (aAlgorithm & (ALGO_SHA256 | ALGO_SHA256_SESS)) { @@ -321,9 +323,13 @@ nsHttpDigestAuth::GenerateCredentials( // returned Authentication-Info header). also used for session info. // nsAutoCString cnonce; - static const char hexChar[] = "0123456789abcdef"; - for (int i = 0; i < 16; ++i) { - cnonce.Append(hexChar[(int)(15.0 * rand() / (RAND_MAX + 1.0))]); + nsTArray<uint8_t> cnonceBuf; + cnonceBuf.SetLength(StaticPrefs::network_http_digest_auth_cnonce_length() / + 2); + PK11_GenerateRandom(reinterpret_cast<unsigned char*>(cnonceBuf.Elements()), + cnonceBuf.Length()); + for (auto byte : cnonceBuf) { + cnonce.AppendPrintf("%02x", byte); } LOG((" cnonce=%s\n", cnonce.get()));

===================================== uriloader/base/nsURILoader.cpp ===================================== @@ -414,28 +414,28 @@ nsresult nsDocumentOpenInfo::DispatchContent(nsIRequest* request) { NS_ASSERTION(!m_targetStreamListener, "If we found a listener, why are we not using it?");

- if (mFlags & nsIURILoader::DONT_RETARGET) { - LOG( - (" External handling forced or (listener not interested and no " - "stream converter exists), and retargeting disallowed -> aborting")); - return NS_ERROR_WONT_HANDLE_CONTENT; - } - // Before dispatching to the external helper app service, check for an HTTP // error page. If we got one, we don't want to handle it with a helper app, // really. - // The WPT a-download-click-404.html requires us to silently handle this - // without displaying an error page, so we just return early here. - // See bug 1604308 for discussion around what the ideal behaviour is. nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(request)); if (httpChannel) { bool requestSucceeded; rv = httpChannel->GetRequestSucceeded(&requestSucceeded); if (NS_FAILED(rv) || !requestSucceeded) { - return NS_OK; + LOG( + (" Returning NS_ERROR_FILE_NOT_FOUND from " + "nsDocumentOpenInfo::DispatchContent due to failed HTTP response")); + return NS_ERROR_FILE_NOT_FOUND; } }

+ if (mFlags & nsIURILoader::DONT_RETARGET) { + LOG( + (" External handling forced or (listener not interested and no " + "stream converter exists), and retargeting disallowed -> aborting")); + return NS_ERROR_WONT_HANDLE_CONTENT; + } + // Fifth step: // // All attempts to dispatch this content have failed. Just pass it off to

View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/e4ed3f3...