diff --git a/toolkit/components/crashes/CrashService.jsm b/toolkit/components/crashes/CrashService.jsm index c433c026e873..c47776435626 100644 --- a/toolkit/components/crashes/CrashService.jsm +++ b/toolkit/components/crashes/CrashService.jsm @@ -65,7 +65,7 @@ function runMinidumpAnalyzer(minidumpPath, allThreads) { break; case "process-failed": gRunningProcesses.delete(process); - resolve(); + reject(); break; default: reject(new Error("Unexpected topic received " + topic)); diff --git a/toolkit/components/telemetry/app/TelemetrySend.jsm b/toolkit/components/telemetry/app/TelemetrySend.jsm index 9d5131058f2d..e1c38ce022c6 100644 --- a/toolkit/components/telemetry/app/TelemetrySend.jsm +++ b/toolkit/components/telemetry/app/TelemetrySend.jsm @@ -315,17 +315,14 @@ var TelemetrySend = { * @param {String} aUrl The telemetry server URL * @param {String} aPingFilePath The path to the file holding the ping * contents, if if sent successfully the pingsender will delete it. - * @param {callback} observer A function called with parameters - (subject, topic, data) and a topic of "process-finished" or - "process-failed" after pingsender completion. * * @throws NS_ERROR_FAILURE if we couldn't find or run the pingsender * executable. * @throws NS_ERROR_NOT_IMPLEMENTED on Android as the pingsender is not * available. */ - testRunPingSender(url, pingPath, observer) { - return TelemetrySendImpl.runPingSender(url, pingPath, observer); + testRunPingSender(url, pingPath) { + TelemetrySendImpl.runPingSender(url, pingPath); }, }; @@ -1556,7 +1553,7 @@ var TelemetrySendImpl = { }; }, - runPingSender(url, pingPath, observer) { + runPingSender(url, pingPath) { if (AppConstants.platform === "android") { throw Cr.NS_ERROR_NOT_IMPLEMENTED; } @@ -1573,6 +1570,6 @@ var TelemetrySendImpl = { process.init(exe); process.startHidden = true; process.noShell = true; - process.runAsync([url, pingPath], 2, observer); + process.run(/* blocking */ false, [url, pingPath], 2); }, }; diff --git a/toolkit/components/telemetry/pingsender/pingsender.cpp b/toolkit/components/telemetry/pingsender/pingsender.cpp index 02c69679105b..e3fd641bd0ff 100644 --- a/toolkit/components/telemetry/pingsender/pingsender.cpp +++ b/toolkit/components/telemetry/pingsender/pingsender.cpp @@ -4,7 +4,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include -#include #include #include #include @@ -26,25 +25,6 @@ const char* kContentEncodingHeader = "Content-Encoding: gzip"; // to the server. const uint32_t kConnectionTimeoutMs = 30 * 1000; -// Operate in std::string because nul bytes will be preserved -bool IsValidDestination(std::string aHost) { - static const std::string kValidDestinations[] = { - "localhost", - "incoming.telemetry.mozilla.org", - }; - for (auto destination : kValidDestinations) { - if (aHost == destination) { - return true; - } - } - return false; -} - -bool IsValidDestination(char* aHost) { - return IsValidDestination(std::string(aHost)); -} - - /** * This shared function returns a Date header string for use in HTTP requests. * See "RFC 7231, section 7.1.1.2: Date" for its specifications. diff --git a/toolkit/components/telemetry/pingsender/pingsender.h b/toolkit/components/telemetry/pingsender/pingsender.h index c0d1f06baaa1..ec69c7784ff2 100644 --- a/toolkit/components/telemetry/pingsender/pingsender.h +++ b/toolkit/components/telemetry/pingsender/pingsender.h @@ -16,7 +16,4 @@ namespace PingSender { // System-specific function to make an HTTP POST operation bool Post(const std::string& url, const std::string& payload); -bool IsValidDestination(char* aUriEndingInHost); -bool IsValidDestination(std::string aUriEndingInHost); - } // namespace PingSender diff --git a/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp b/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp index 3a4580eeb6d0..b7e99c4e4311 100644 --- a/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp +++ b/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp @@ -3,7 +3,6 @@ * 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 #include #include #include @@ -27,7 +26,6 @@ class CurlWrapper { CurlWrapper(); ~CurlWrapper(); bool Init(); - bool IsValidDestination(const string& url); bool Post(const string& url, const string& payload); // libcurl functions @@ -41,16 +39,9 @@ class CurlWrapper { void (*easy_cleanup)(CURL*); void (*global_cleanup)(void); - CURLU* (*curl_url)(); - CURLUcode (*curl_url_get)(CURLU *, CURLUPart, char **, unsigned int); - CURLUcode (*curl_url_set)(CURLU *, CURLUPart, const char *, unsigned int); - void (*curl_free)( char *); - void (*curl_url_cleanup)(CURLU *); - private: void* mLib; void* mCurl; - bool mCanParseUrl; }; CurlWrapper::CurlWrapper() @@ -63,11 +54,6 @@ CurlWrapper::CurlWrapper() easy_strerror(nullptr), easy_cleanup(nullptr), global_cleanup(nullptr), - curl_url(nullptr), - curl_url_get(nullptr), - curl_url_set(nullptr), - curl_free(nullptr), - curl_url_cleanup(nullptr), mLib(nullptr), mCurl(nullptr) {} @@ -129,12 +115,6 @@ bool CurlWrapper::Init() { *(void**)(&easy_cleanup) = dlsym(mLib, "curl_easy_cleanup"); *(void**)(&global_cleanup) = dlsym(mLib, "curl_global_cleanup"); - *(void**)(&curl_url) = dlsym(mLib, "curl_url"); - *(void**)(&curl_url_set) = dlsym(mLib, "curl_url_set"); - *(void**)(&curl_url_get) = dlsym(mLib, "curl_url_get"); - *(void**)(&curl_free) = dlsym(mLib, "curl_free"); - *(void**)(&curl_url_cleanup) = dlsym(mLib, "curl_url_cleanup"); - if (!easy_init || !easy_setopt || !easy_perform || !easy_getinfo || !slist_append || !slist_free_all || !easy_strerror || !easy_cleanup || !global_cleanup) { @@ -142,13 +122,6 @@ bool CurlWrapper::Init() { return false; } - mCanParseUrl = true; - if (!curl_url || !curl_url_get || !curl_url_set || !curl_free || - !curl_url_cleanup) { - mCanParseUrl = false; - PINGSENDER_LOG("WARNING: Do not have url parsing functions in libcurl\n"); - } - mCurl = easy_init(); if (!mCurl) { @@ -169,67 +142,6 @@ static size_t DummyWriteCallback(char* ptr, size_t size, size_t nmemb, return size * nmemb; } -// If we can't use curl's URL parsing (which is safer) we have to fallback -// to this handwritten one (which is only as safe as we are clever.) -bool FallbackIsValidDestination(const string& aUrl) { - // Lowercase the url - string url = aUrl; - std::transform(url.begin(), url.end(), url.begin(), - [](unsigned char c){ return std::tolower(c); }); - // Strip off the scheme in the beginning - if (url.find_first_of("http://") != std::string::npos) { - url = url.substr(7); - } else if (url.find_first_of("https://") != std::string::npos) { - url = url.substr(8); - } - - // Remove any user information. If a @ appeared in the userinformation, - // it would need to be encoded. - unsigned long atStart = url.find_first_of("@"); - url = (atStart == std::string::npos) ? url : url.substr(atStart + 1); - - // Remove any path or fragment information, leaving us with a url that may contain - // host, and port. - unsigned long fragStart = url.find_first_of("#"); - url = (fragStart == std::string::npos) ? url : url.substr(0, fragStart); - unsigned long pathStart = url.find_first_of("/"); - url = (pathStart == std::string::npos) ? url : url.substr(0, pathStart); - - // Remove the port, because we run tests targeting localhost:port - unsigned long portStart = url.find_last_of(":"); - url = (portStart == std::string::npos) ? url : url.substr(0, portStart); - - return ::IsValidDestination(url); -} - -bool CurlWrapper::IsValidDestination(const string& aUrl) { - if (!mCanParseUrl) { - return FallbackIsValidDestination(aUrl); - } - - bool ret = false; - CURLU *h = curl_url(); - if (!h) { - return FallbackIsValidDestination(aUrl); - } - - if (CURLUE_OK != curl_url_set(h, CURLUPART_URL, aUrl.c_str(), 0)) { - goto cleanup; - } - - char *host; - if (CURLUE_OK != curl_url_get(h, CURLUPART_HOST, &host, 0)) { - goto cleanup; - } - - ret = ::IsValidDestination(host); - curl_free(host); - -cleanup: - curl_url_cleanup(h); - return ret; -} - bool CurlWrapper::Post(const string& url, const string& payload) { easy_setopt(mCurl, CURLOPT_URL, url.c_str()); easy_setopt(mCurl, CURLOPT_USERAGENT, kUserAgent); @@ -284,10 +196,6 @@ bool Post(const string& url, const string& payload) { if (!curl.Init()) { return false; } - if (!curl.IsValidDestination(url)) { - PINGSENDER_LOG("ERROR: Invalid destination host\n"); - return false; - } return curl.Post(url, payload); } diff --git a/toolkit/components/telemetry/pingsender/pingsender_win.cpp b/toolkit/components/telemetry/pingsender/pingsender_win.cpp index 118f1accadd0..6d256dadf54f 100644 --- a/toolkit/components/telemetry/pingsender/pingsender_win.cpp +++ b/toolkit/components/telemetry/pingsender/pingsender_win.cpp @@ -61,11 +61,6 @@ bool Post(const string& url, const string& payload) { return false; } - if (!IsValidDestination(host)) { - PINGSENDER_LOG("ERROR: Invalid destination host '%s'\n", host); - return false; - } - ScopedHInternet internet(InternetOpen(kUserAgent, INTERNET_OPEN_TYPE_PRECONFIG, /* lpszProxyName */ NULL, diff --git a/toolkit/components/telemetry/tests/unit/test_PingSender.js b/toolkit/components/telemetry/tests/unit/test_PingSender.js index 0c62e020ff71..c48fef71d354 100644 --- a/toolkit/components/telemetry/tests/unit/test_PingSender.js +++ b/toolkit/components/telemetry/tests/unit/test_PingSender.js @@ -96,24 +96,7 @@ add_task(async function test_pingSender() { // Try to send it using the pingsender. const url = "http://localhost:" + PingServer.port + "/submit/telemetry/"; - TelemetrySend.testRunPingSender(url, pingPath, (_, topic, __) => { - switch (topic) { - case "process-finished": // finished indicates an exit code of 0 - Assert.equal( - true, - true, - "Pingsender should be able to post to localhost" - ); - break; - case "process-failed": // failed indicates an exit code != 0 - Assert.equal( - true, - false, - "Pingsender should be able to post to localhost" - ); - break; - } - }); + TelemetrySend.testRunPingSender(url, pingPath); let req = await PingServer.promiseNextRequest(); let ping = decodeRequestPayload(req); @@ -149,42 +132,6 @@ add_task(async function test_pingSender() { // Check that the PingSender removed the pending ping. await waitForPingDeletion(data.id); - // Confirm we can't send a ping to another destination url - let bannedUris = [ - "https://example.com", - "http://localhost.com", - "http://localHOST.com", - "http://localhost@example.com", - "http://localhost:bob@example.com", - "http://localhost:localhost@localhost.example.com", - ]; - for (let indx in bannedUris) { - TelemetrySend.testRunPingSender( - bannedUris[indx], - pingPath, - (_, topic, __) => { - switch (topic) { - case "process-finished": // finished indicates an exit code of 0 - Assert.equal( - false, - true, - "Pingsender should not be able to post to any banned urls: " + - bannedUris[indx] - ); - break; - case "process-failed": // failed indicates an exit code != 0 - Assert.equal( - true, - true, - "Pingsender should not be able to post to any banned urls: " + - bannedUris[indx] - ); - break; - } - } - ); - } - // Shut down the failing server. We do this now, and not right after using it, // to make sure we're not interfering with the test. await new Promise(r => failingServer.stop(r)); diff --git a/xpcom/threads/nsProcessCommon.cpp b/xpcom/threads/nsProcessCommon.cpp index 2942fbf9e823..b626de205357 100644 --- a/xpcom/threads/nsProcessCommon.cpp +++ b/xpcom/threads/nsProcessCommon.cpp @@ -312,7 +312,7 @@ void nsProcess::ProcessComplete() { } const char* topic; - if (mExitValue != 0) { + if (mExitValue < 0) { topic = "process-failed"; } else { topic = "process-finished";