From d3b9fc8dde6053c1153637936fb11978cc5bb1fa Mon Sep 17 00:00:00 2001 From: Ciure Andrei Date: Thu, 16 Apr 2020 23:54:06 +0300 Subject: [PATCH] Backed out changeset c424381097d7 (bug 1627206) for causing browser_console_logging.js failures CLOSED TREE --- dom/security/nsHTTPSOnlyStreamListener.cpp | 135 ++++-------------- dom/security/nsHTTPSOnlyStreamListener.h | 17 +-- .../https-only/browser_console_logging.js | 4 +- .../test/https-only/file_console_logging.html | 2 +- toolkit/components/telemetry/Histograms.json | 35 ----- 5 files changed, 32 insertions(+), 161 deletions(-) diff --git a/dom/security/nsHTTPSOnlyStreamListener.cpp b/dom/security/nsHTTPSOnlyStreamListener.cpp index 73ea5d792f69..8e7702bf51bc 100644 --- a/dom/security/nsHTTPSOnlyStreamListener.cpp +++ b/dom/security/nsHTTPSOnlyStreamListener.cpp @@ -4,27 +4,22 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include "nsHTTPSOnlyUtils.h" -#include "NSSErrorsService.h" -#include "mozilla/Telemetry.h" -#include "mozilla/TimeStamp.h" -#include "mozpkix/pkixnss.h" -#include "nsCOMPtr.h" #include "nsHTTPSOnlyStreamListener.h" +#include "nsHTTPSOnlyUtils.h" +#include "nsCOMPtr.h" #include "nsIChannel.h" #include "nsIRequest.h" #include "nsITransportSecurityInfo.h" #include "nsIURI.h" #include "nsPrintfCString.h" -#include "secerr.h" -#include "sslerr.h" NS_IMPL_ISUPPORTS(nsHTTPSOnlyStreamListener, nsIStreamListener, nsIRequestObserver) nsHTTPSOnlyStreamListener::nsHTTPSOnlyStreamListener( - nsIStreamListener* aListener) - : mListener(aListener), mCreationStart(mozilla::TimeStamp::Now()) {} + nsIStreamListener* aListener) { + mListener = aListener; +} NS_IMETHODIMP nsHTTPSOnlyStreamListener::OnDataAvailable(nsIRequest* aRequest, @@ -41,103 +36,31 @@ nsHTTPSOnlyStreamListener::OnStartRequest(nsIRequest* request) { NS_IMETHODIMP nsHTTPSOnlyStreamListener::OnStopRequest(nsIRequest* request, nsresult aStatus) { - // DNS errors are unrelated to the HTTPS-Only mode, so they can be ignored. - if (aStatus != NS_ERROR_UNKNOWN_HOST) { - RecordUpgradeTelemetry(request, aStatus); - LogUpgradeFailure(request, aStatus); + // If the request failed we'll log it to the console with the error-code + if (NS_FAILED(aStatus)) { + nsresult rv; + // Try to query for the channel-object + nsCOMPtr channel = do_QueryInterface(request, &rv); + if (NS_SUCCEEDED(rv)) { + nsCOMPtr loadInfo = channel->LoadInfo(); + uint32_t innerWindowId = loadInfo->GetInnerWindowID(); + + nsCOMPtr uri; + rv = channel->GetURI(getter_AddRefs(uri)); + if (NS_SUCCEEDED(rv)) { + // Logging URI as well as Module- and Error-Code + AutoTArray params = { + NS_ConvertUTF8toUTF16(uri->GetSpecOrDefault()), + NS_ConvertUTF8toUTF16(nsPrintfCString("M%u-C%u", + NS_ERROR_GET_MODULE(aStatus), + NS_ERROR_GET_CODE(aStatus)))}; + nsHTTPSOnlyUtils::LogLocalizedString( + "HTTPSOnlyFailedRequest", params, nsIScriptError::errorFlag, + innerWindowId, !!loadInfo->GetOriginAttributes().mPrivateBrowsingId, + uri); + } + } } return mListener->OnStopRequest(request, aStatus); } - -void nsHTTPSOnlyStreamListener::RecordUpgradeTelemetry(nsIRequest* request, - nsresult aStatus) { - // 1. Get time between now and when the initial upgrade request started - int64_t duration = - (mozilla::TimeStamp::Now() - mCreationStart).ToMilliseconds(); - - // 2. Assemble the category string - // [!] All strings have to be present in Histograms.json - nsresult rv; - nsCOMPtr channel = do_QueryInterface(request, &rv); - if (NS_FAILED(rv)) { - return; - } - - nsAutoCString category; - nsCOMPtr loadInfo = channel->LoadInfo(); - - if (loadInfo->InternalContentPolicyType() == - nsIContentPolicy::TYPE_DOCUMENT) { - category.AppendLiteral("top_"); - } else { - category.AppendLiteral("sub_"); - } - - if (NS_SUCCEEDED(aStatus)) { - category.AppendLiteral("successful"); - } else { - int32_t code = -1 * NS_ERROR_GET_CODE(aStatus); - - if (aStatus == NS_ERROR_REDIRECT_LOOP) { - category.AppendLiteral("f_redirectloop"); - } else if (aStatus == NS_ERROR_NET_TIMEOUT) { - category.AppendLiteral("f_timeout"); - } else if (aStatus == NS_BINDING_ABORTED) { - category.AppendLiteral("f_aborted"); - } else if (aStatus == NS_ERROR_CONNECTION_REFUSED) { - category.AppendLiteral("f_cxnrefused"); - } else if (mozilla::psm::IsNSSErrorCode(code)) { - switch (code) { - case mozilla::pkix::MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT: - category.AppendLiteral("f_ssl_selfsignd"); - break; - case SSL_ERROR_BAD_CERT_DOMAIN: - category.AppendLiteral("f_ssl_badcertdm"); - break; - case SEC_ERROR_UNKNOWN_ISSUER: - category.AppendLiteral("f_ssl_unkwnissr"); - break; - default: - category.AppendLiteral("f_ssl_other"); - break; - } - } else { - category.AppendLiteral("f_other"); - } - } - - mozilla::Telemetry::Accumulate( - mozilla::Telemetry::HTTPS_ONLY_MODE_UPGRADE_TIME_MS, category, duration); -} - -void nsHTTPSOnlyStreamListener::LogUpgradeFailure(nsIRequest* request, - nsresult aStatus) { - // If the request failed we'll log it to the console with the error-code - if (NS_SUCCEEDED(aStatus)) { - return; - } - nsresult rv; - // Try to query for the channel-object - nsCOMPtr channel = do_QueryInterface(request, &rv); - if (NS_FAILED(rv)) { - return; - } - nsCOMPtr loadInfo = channel->LoadInfo(); - uint32_t innerWindowId = loadInfo->GetInnerWindowID(); - - nsCOMPtr uri; - rv = channel->GetURI(getter_AddRefs(uri)); - if (NS_FAILED(rv)) { - return; - } - // Logging URI as well as Module- and Error-Code - AutoTArray params = { - NS_ConvertUTF8toUTF16(uri->GetSpecOrDefault()), - NS_ConvertUTF8toUTF16(nsPrintfCString("M%u-C%u", - NS_ERROR_GET_MODULE(aStatus), - NS_ERROR_GET_CODE(aStatus)))}; - nsHTTPSOnlyUtils::LogLocalizedString( - "HTTPSOnlyFailedRequest", params, nsIScriptError::errorFlag, - innerWindowId, !!loadInfo->GetOriginAttributes().mPrivateBrowsingId, uri); -} diff --git a/dom/security/nsHTTPSOnlyStreamListener.h b/dom/security/nsHTTPSOnlyStreamListener.h index d3255c5b2261..b7598ab58eed 100644 --- a/dom/security/nsHTTPSOnlyStreamListener.h +++ b/dom/security/nsHTTPSOnlyStreamListener.h @@ -7,9 +7,8 @@ #ifndef nsHTTPSOnlyStreamListener_h___ #define nsHTTPSOnlyStreamListener_h___ -#include "mozilla/TimeStamp.h" -#include "nsCOMPtr.h" #include "nsIStreamListener.h" +#include "nsCOMPtr.h" /** * This event listener gets registered for requests that have been upgraded @@ -27,21 +26,7 @@ class nsHTTPSOnlyStreamListener : public nsIStreamListener { private: virtual ~nsHTTPSOnlyStreamListener() = default; - /** - * Records telemetry about the upgraded request. - * @param aStatus Request object - */ - void RecordUpgradeTelemetry(nsIRequest* request, nsresult aStatus); - - /** - * Logs information to the console if the request failed. - * @param request Request object - * @param aStatus Status of request - */ - void LogUpgradeFailure(nsIRequest* request, nsresult aStatus); - nsCOMPtr mListener; - mozilla::TimeStamp mCreationStart; }; #endif /* nsHTTPSOnlyStreamListener_h___ */ diff --git a/dom/security/test/https-only/browser_console_logging.js b/dom/security/test/https-only/browser_console_logging.js index 46827a5c11e7..469faa733e4b 100644 --- a/dom/security/test/https-only/browser_console_logging.js +++ b/dom/security/test/https-only/browser_console_logging.js @@ -57,11 +57,9 @@ const testPathUpgradeable = getRootDirectory(gTestPath).replace( "chrome://mochitests/content", "http://example.com" ); -// DNS errors are not logged as HTTPS-Only Mode upgrade failures, so we have to -// upgrade to a domain that exists but fails. const testPathNotUpgradeable = getRootDirectory(gTestPath).replace( "chrome://mochitests/content", - "http://nocert.example.com" + "http://mochi.test:8888" ); const kTestURISuccess = testPathUpgradeable + "file_console_logging.html"; const kTestURIFail = testPathNotUpgradeable + "file_console_logging.html"; diff --git a/dom/security/test/https-only/file_console_logging.html b/dom/security/test/https-only/file_console_logging.html index a9bc9cf59da5..fbaac9c5f6d2 100644 --- a/dom/security/test/https-only/file_console_logging.html +++ b/dom/security/test/https-only/file_console_logging.html @@ -9,6 +9,6 @@ - + diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index bdead80eb4a4..ef4c1178cfe5 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -5678,41 +5678,6 @@ "kind": "count", "description": "Number of unique pages that contain an unsafe-eval CSP directive" }, - "HTTPS_ONLY_MODE_UPGRADE_TIME_MS": { - "record_in_processes": ["main"], - "products": ["firefox", "geckoview"], - "alert_emails": ["julianwels@mozilla.com", "seceng-telemetry@mozilla.com"], - "bug_numbers": [1627206], - "expires_in_version": "never", - "kind": "exponential", - "low": 50, - "high": 300000, - "n_buckets": 30, - "keyed": true, - "keys": [ - "top_successful", - "sub_successful", - "top_f_redirectloop", - "sub_f_redirectloop", - "top_f_timeout", - "sub_f_timeout", - "top_f_aborted", - "sub_f_aborted", - "top_f_cxnrefused", - "sub_f_cxnrefused", - "top_f_ssl_selfsignd", - "sub_f_ssl_selfsignd", - "top_f_ssl_badcertdm", - "sub_f_ssl_badcertdm", - "top_f_ssl_unkwnissr", - "sub_f_ssl_unkwnissr", - "top_f_ssl_other", - "sub_f_ssl_other", - "top_f_other", - "sub_f_other" - ], - "description": "Time it takes for a request that has been upgraded with HTTPS-Only Mode to complete, broken down by top-level (top) / sub-resource (sub) and status" - }, "PLACES_DATABASE_CORRUPTION_HANDLING_STAGE": { "record_in_processes": ["main"], "products": ["firefox", "fennec", "geckoview", "thunderbird"],