diff --git a/crash_reporter/chrome_collector.cc b/crash_reporter/chrome_collector.cc index d8250f73f..2c9ae6e81 100644 --- a/crash_reporter/chrome_collector.cc +++ b/crash_reporter/chrome_collector.cc @@ -45,6 +45,10 @@ bool ChromeCollector::HandleCrash(const std::string &file_path, if (!is_feedback_allowed_function_()) return true; + if (exe_name.find('/') != std::string::npos) { + LOG(ERROR) << "exe_name contains illegal characters: " << exe_name; + return false; + } FilePath dir; uid_t uid = atoi(uid_string.c_str()); @@ -65,7 +69,7 @@ bool ChromeCollector::HandleCrash(const std::string &file_path, return false; } - if (!ParseCrashLog(data, dir, minidump_path)) { + if (!ParseCrashLog(data, dir, minidump_path, dump_basename)) { LOG(ERROR) << "Failed to parse Chrome's crash log"; return false; } @@ -81,27 +85,37 @@ bool ChromeCollector::HandleCrash(const std::string &file_path, bool ChromeCollector::ParseCrashLog(const std::string &data, const FilePath &dir, - const FilePath &minidump) { + const FilePath &minidump, + const std::string &basename) { size_t at = 0; while (at < data.size()) { // Look for a : followed by a decimal number, followed by another : // followed by N bytes of data. std::string name, size_string; - if (!GetDelimitedString(data, ':', at, &name)) + if (!GetDelimitedString(data, ':', at, &name)) { + LOG(ERROR) << "Can't find : after name @ offset " << at; break; + } at += name.size() + 1; // Skip the name & : delimiter. - if (!GetDelimitedString(data, ':', at, &size_string)) + if (!GetDelimitedString(data, ':', at, &size_string)) { + LOG(ERROR) << "Can't find : after size @ offset " << at; break; + } at += size_string.size() + 1; // Skip the size & : delimiter. size_t size; - if (!base::StringToSizeT(size_string, &size)) + if (!base::StringToSizeT(size_string, &size)) { + LOG(ERROR) << "String not convertible to integer: " << size_string; break; + } // Data would run past the end, did we get a truncated file? - if (at + size > data.size()) + if (at + size > data.size()) { + LOG(ERROR) << "Overrun, expected " << size << " bytes of data, got " + << (data.size() - at); break; + } if (name.find("filename") != std::string::npos) { // File. @@ -110,19 +124,19 @@ bool ChromeCollector::ParseCrashLog(const std::string &data, // Descriptive name will be upload_file_minidump for the dump. std::string desc, filename; pcrecpp::RE re("(.*)\" *; *filename=\"(.*)\""); - if (!re.FullMatch(name.c_str(), &desc, &filename)) + if (!re.FullMatch(name.c_str(), &desc, &filename)) { + LOG(ERROR) << "Filename was not in expected format: " << name; break; + } - if (filename.compare(kDefaultMinidumpName) == 0) { + if (desc.compare(kDefaultMinidumpName) == 0) { // The minidump. WriteNewFile(minidump, data.c_str() + at, size); } else { // Some other file. - FilePath path = GetCrashPath(dir, filename, "other"); + FilePath path = GetCrashPath(dir, basename + "-" + filename, "other"); if (WriteNewFile(path, data.c_str() + at, size) >= 0) { - std::string value = "@"; - value.append(path.value()); - AddCrashMetaData(desc, value); + AddCrashMetaUploadFile(desc, path.value()); } } } else { @@ -160,7 +174,7 @@ bool ChromeCollector::ParseCrashLog(const std::string &data, break; } } - AddCrashMetaData(name, value_str); + AddCrashMetaUploadData(name, value_str); } at += size; diff --git a/crash_reporter/chrome_collector.h b/crash_reporter/chrome_collector.h index e25cd86a6..0a8a95671 100644 --- a/crash_reporter/chrome_collector.h +++ b/crash_reporter/chrome_collector.h @@ -37,7 +37,8 @@ class ChromeCollector : public CrashCollector { // For file values, name actually contains both a description and a filename, // in a fixed format of: "; filename="" bool ParseCrashLog(const std::string &data, const base::FilePath &dir, - const base::FilePath &minidump); + const base::FilePath &minidump, + const std::string &basename); }; #endif diff --git a/crash_reporter/chrome_collector_test.cc b/crash_reporter/chrome_collector_test.cc index b24a194cc..9a5ee9bf5 100644 --- a/crash_reporter/chrome_collector_test.cc +++ b/crash_reporter/chrome_collector_test.cc @@ -68,7 +68,8 @@ class ChromeCollectorTest : public ::testing::Test { TEST_F(ChromeCollectorTest, GoodValues) { FilePath dir("."); EXPECT_TRUE(collector_.ParseCrashLog(kCrashFormatGood, - dir, dir.Append("minidump.dmp"))); + dir, dir.Append("minidump.dmp"), + "base")); // Check to see if the values made it in properly. std::string meta = collector_.extra_metadata_; @@ -79,7 +80,8 @@ TEST_F(ChromeCollectorTest, GoodValues) { TEST_F(ChromeCollectorTest, Newlines) { FilePath dir("."); EXPECT_TRUE(collector_.ParseCrashLog(kCrashFormatEmbeddedNewline, - dir, dir.Append("minidump.dmp"))); + dir, dir.Append("minidump.dmp"), + "base")); // Check to see if the values were escaped. std::string meta = collector_.extra_metadata_; @@ -101,14 +103,16 @@ TEST_F(ChromeCollectorTest, BadValues) { for (size_t i = 0; i < sizeof(list) / sizeof(list[0]); i++) { chromeos::ClearLog(); EXPECT_FALSE(collector_.ParseCrashLog(list[i].data, - dir, dir.Append("minidump.dmp"))); + dir, dir.Append("minidump.dmp"), + "base")); } } TEST_F(ChromeCollectorTest, File) { FilePath dir("."); EXPECT_TRUE(collector_.ParseCrashLog(kCrashFormatWithFile, - dir, dir.Append("minidump.dmp"))); + dir, dir.Append("minidump.dmp"), + "base")); // Check to see if the values are still correct and that the file was // written with the right data. @@ -116,8 +120,9 @@ TEST_F(ChromeCollectorTest, File) { EXPECT_TRUE(meta.find("value1=abcdefghij") != std::string::npos); EXPECT_TRUE(meta.find("value2=12345") != std::string::npos); EXPECT_TRUE(meta.find("value3=ok") != std::string::npos); - ExpectFileEquals("12345\n789\n12345", "foo.txt.other"); - file_util::Delete(dir.Append("foo.txt.other"), false); + ExpectFileEquals("12345\n789\n12345", + dir.Append("base-foo.txt.other").value().c_str()); + file_util::Delete(dir.Append("base-foo.txt.other"), false); } int main(int argc, char **argv) { diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc index b433bec65..d39d6f48a 100644 --- a/crash_reporter/crash_collector.cc +++ b/crash_reporter/crash_collector.cc @@ -39,6 +39,8 @@ static const char kLeaveCoreFile[] = "/root/.leave_core"; static const char kLsbRelease[] = "/etc/lsb-release"; static const char kShellPath[] = "/bin/sh"; static const char kSystemCrashPath[] = "/var/spool/crash"; +static const char kUploadVarPrefix[] = "upload_var_"; +static const char kUploadFilePrefix[] = "upload_file_"; // Normally this path is not used. Unfortunately, there are a few edge cases // where we need this. Any process that runs as kDefaultUserName that crashes // is consider a "user crash". That includes the initial Chrome browser that @@ -479,6 +481,18 @@ void CrashCollector::AddCrashMetaData(const std::string &key, extra_metadata_.append(StringPrintf("%s=%s\n", key.c_str(), value.c_str())); } +void CrashCollector::AddCrashMetaUploadFile(const std::string &key, + const std::string &path) { + if (!path.empty()) + AddCrashMetaData(kUploadFilePrefix + key, path); +} + +void CrashCollector::AddCrashMetaUploadData(const std::string &key, + const std::string &value) { + if (!value.empty()) + AddCrashMetaData(kUploadVarPrefix + key, value); +} + void CrashCollector::WriteCrashMetaData(const FilePath &meta_path, const std::string &exec_name, const std::string &payload_path) { diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h index 4cbe1b977..19ec97b03 100644 --- a/crash_reporter/crash_collector.h +++ b/crash_reporter/crash_collector.h @@ -137,6 +137,16 @@ class CrashCollector { // "\n" characters. Value must not contain "\n" characters. void AddCrashMetaData(const std::string &key, const std::string &value); + // Add a file to be uploaded to the crash reporter server. The file must + // persist until the crash report is sent; ideally it should live in the same + // place as the .meta file, so it can be cleaned up automatically. + void AddCrashMetaUploadFile(const std::string &key, const std::string &path); + + // Add non-standard meta data to the crash metadata file. + // Data added though this call will be uploaded to the crash reporter server, + // appearing as a form field. + void AddCrashMetaUploadData(const std::string &key, const std::string &value); + // Write a file of metadata about crash. void WriteCrashMetaData(const base::FilePath &meta_path, const std::string &exec_name, diff --git a/crash_reporter/crash_sender b/crash_reporter/crash_sender index 6bebb6ea8..72205cffc 100755 --- a/crash_reporter/crash_sender +++ b/crash_reporter/crash_sender @@ -240,16 +240,25 @@ get_kind() { } get_key_value() { - local file=$1 key=$2 value + local file="$1" key="$2" value if [ -f "${file}" ]; then # Return the first entry. There shouldn't be more than one anyways. - value=$(awk -F= '/^'"${key}"'[[:space:]]*=/ { print $NF }' "${file}") + # Substr at length($1) + 2 skips past the key and following = sign (awk + # uses 1-based indexes), but preserves embedded = characters. + value=$(sed -n "/^${key}[[:space:]]*=/{s:^[^=]*=::p;q}" "${file}") fi echo "${value:-undefined}" } +get_keys() { + local file="$1" regex="$2" + + awk -F'[[:space:]=]' -vregex="${regex}" \ + 'match($1, regex) { print $1 }' "${file}" +} + # Return the board name. get_board() { get_key_value "/etc/lsb-release" "CHROMEOS_RELEASE_BOARD" @@ -286,6 +295,46 @@ send_crash() { local log="$(get_key_value "${meta_path}" "log")" local sig="$(get_key_value "${meta_path}" "sig")" local send_payload_size="$(stat --printf=%s "${report_payload}" 2>/dev/null)" + local product="$(get_key_value "${meta_path}" "upload_var_prod")" + local version="$(get_key_value "${meta_path}" "upload_var_ver")" + local upload_prefix="$(get_key_value "${meta_path}" "upload_prefix")" + + set -- \ + -F "write_payload_size=${write_payload_size}" \ + -F "send_payload_size=${send_payload_size}" + if [ "${sig}" != "undefined" ]; then + set -- "$@" \ + -F "sig=${sig}" \ + -F "sig2=${sig}" + fi + if [ "${log}" != "undefined" ]; then + set -- "$@" \ + -F "log=@${log}" + fi + + if [ "${upload_prefix}" = "undefined" ]; then + upload_prefix="" + fi + + # Grab any variable that begins with upload_. + local v + for k in $(get_keys "${meta_path}" "^upload_"); do + v="$(get_key_value "${meta_path}" "${k}")" + case ${k} in + # Product & version are handled separately. + upload_var_prod) ;; + upload_var_ver) ;; + upload_var_*) set -- "$@" -F "${upload_prefix}${k#upload_var_}=${v}" ;; + upload_file_*) set -- "$@" -F "${upload_prefix}${k#upload_file_}=@${v}" ;; + esac + done + + # When uploading Chrome reports we need to report the right product and + # version, so allow the meta file to override these values. + if [ "${product}" = "undefined" ]; then + product=${CHROMEOS_PRODUCT} + version=${chromeos_version} + fi local image_type if is_test_image; then @@ -307,38 +356,32 @@ send_crash() { boot_mode="dev" fi - local extra_key1="write_payload_size" - local extra_value1="${write_payload_size}" - local extra_key2="send_payload_size" - local extra_value2="${send_payload_size}" - if [ "${sig}" != "undefined" ]; then - extra_key1="sig" - extra_value1="${sig}" - extra_key2="sig2" - extra_value2="${sig}" - elif [ "${log}" != "undefined" ]; then - # Upload a log file if it was specified. - extra_key1="log" - extra_value1="@${log}" - fi - local error_type="$(get_key_value "${meta_path}" "error_type")" [ "${error_type}" = "undefined" ] && error_type= lecho "Sending crash:" + if [ "${product}" != "${CHROMEOS_PRODUCT}" ]; then + lecho " Sending crash report on behalf of ${product}" + fi lecho " Scheduled to send in ${sleep_time}s" lecho " Metadata: ${meta_path} (${kind})" lecho " Payload: ${report_payload}" - lecho " Version: ${chromeos_version}" + lecho " Version: ${version}" [ -n "${image_type}" ] && lecho " Image type: ${image_type}" [ -n "${boot_mode}" ] && lecho " Boot mode: ${boot_mode}" if is_mock; then - lecho " Product: ${CHROMEOS_PRODUCT}" + lecho " Product: ${product}" lecho " URL: ${url}" lecho " Board: ${board}" lecho " HWClass: ${hwclass}" - lecho " ${extra_key1}: ${extra_value1}" - lecho " ${extra_key2}: ${extra_value2}" + lecho " write_payload_size: ${write_payload_size}" + lecho " send_payload_size: ${send_payload_size}" + if [ "${log}" != "undefined" ]; then + lecho " log: @${log}" + fi + if [ "${sig}" != "undefined" ]; then + lecho " sig: ${sig}" + fi fi lecho " Exec name: ${exec_name}" [ -n "${error_type}" ] && lecho " Error type: ${error_type}" @@ -369,8 +412,8 @@ send_crash() { set +e curl "${url}" ${proxy:+--proxy "$proxy"} \ --capath "${RESTRICTED_CERTIFICATES_PATH}" --ciphers HIGH \ - -F "prod=${CHROMEOS_PRODUCT}" \ - -F "ver=${chromeos_version}" \ + -F "prod=${product}" \ + -F "ver=${version}" \ -F "upload_file_${kind}=@${report_payload}" \ -F "board=${board}" \ -F "hwclass=${hwclass}" \ @@ -378,9 +421,10 @@ send_crash() { ${image_type:+-F "image_type=${image_type}"} \ ${boot_mode:+-F "boot_mode=${boot_mode}"} \ ${error_type:+-F "error_type=${error_type}"} \ - -F "${extra_key1}=${extra_value1}" \ - -F "${extra_key2}=${extra_value2}" \ - -F "guid=<${CONSENT_ID}" -o "${report_id}" 2>"${curl_stderr}" + -F "guid=<${CONSENT_ID}" \ + -o "${report_id}" \ + "$@" \ + 2>"${curl_stderr}" curl_result=$? set -e