89 Commits

Author SHA1 Message Date
Francois Rousseau
27322abb7e [net] specify parameter name in parameterized test suite
* this renders as "HTTPTransport/HTTPTransport.ValidFormData_Gzip/http"
  instead of the default "HTTPTransport/HTTPTransport.ValidFormData_Gzip/0"
* switch the parameter type from a base::FilePath::StringType to a string

Change-Id: I19743966406f92176c566827d74a79aef5a87bb5
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1900324
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Francois Rousseau <frousseau@google.com>
2019-11-06 01:35:18 +00:00
Joshua Peraza
1b60c8172c commit test ssl certificate to testdata
Chromium requires build artifacts to be generated deterministically so
commit a long-lived (10 years) test certificate to the repository.

Change-Id: I7a6e2441f506196ca58fbbf757648fa0ac70bc9a
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1872188
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2019-11-05 17:47:01 +00:00
Joshua Peraza
47a342133d linux: use boringssl in chromium
Or else the uploader will check fail when uploading to https://...

Change-Id: I88a765215cc7bff5809b8effd92f4e39bebd1e5b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1860940
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2019-10-15 15:58:26 +00:00
Joshua Peraza
d98690a182 android: enable upload
This patch also updates WorkerThread to execute DoWork() when
DoWorkNow() has been called, which is relevant when DoWorkNow() and
Stop() have both been called. This occurs regularly on Android where
the handler's current normal mode is to dump a single process and exit.
This change ensures the upload thread has a chance to upload the report
before the handler exits.

This change should not affect upload on Chrome/WebView/Chromecast which
don't pass Crashpad a --url option and are still responsible for their
own uploads.

Change-Id: Ie5553eafc13714f0438b4b133a92516f7abec153
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1643710
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-06-18 22:07:14 +00:00
Joshua Peraza
7d5d5ff25f Avoid triggering buffer overflow detectors
Writing directly into buf.tmp causes the nul-terminator to overflow
into buf.crlf, which upsets some overflow detectors.

Bug: crashpad:289
Change-Id: I241f1ae239ed8360ac5dfd245cb70e919ae73cd1
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1545014
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2019-03-29 15:34:02 +00:00
Mark Mentovai
6a209070e4 Change deprecated gtest TEST_CASE macros to TEST_SUITE
No functional change. See
https://github.com/google/googletest/blob/master/googletest/docs/primer.md#beware-of-the-nomenclature
(as of 5d3a2cd9c854).

Change-Id: I0f6dc59f014b01d18a09a92f016351a7402d8e6c
Reviewed-on: https://chromium-review.googlesource.com/c/1427499
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2019-01-22 20:58:58 +00:00
Mark Mentovai
cc166d71f4 Use base::size where appropriate, and ArraySize elsewhere
This is a follow-up to c8a016b99d97, following the post-landing
discussion at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1393921/5#message-2058541d8c4505d20a990ab7734cd758e437a5f7

base::size, and std::size that will eventually replace it when C++17 is
assured, does not allow the size of non-static data members to be taken
in constant expression context. The remaining uses of ArraySize are in:

minidump/minidump_exception_writer.cc (×1)
minidump/minidump_system_info_writer.cc (×2, also uses base::size)
snapshot/cpu_context.cc (×4, also uses base::size)
util/misc/arraysize_test.cc (×10, of course)

The first of these occurs when initializing a constexpr variable. All
others are in expressions used with static_assert.

Includes:
Update mini_chromium to 737433ebade4d446643c6c07daae02a67e8deccao

f701716d9546 Add Windows ARM64 build target to mini_chromium
87a95a3d6ac2 Remove the arraysize macro
1f7255ead1f7 Placate MSVC in areas of base::size usage
737433ebade4 Add cast

Bug: chromium:837308
Change-Id: I6a5162654461b1bdd9b7b6864d0d71a734bcde19
Reviewed-on: https://chromium-review.googlesource.com/c/1396108
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-01-04 22:42:57 +00:00
Avi Drissman
c8a016b99d Remove base's arraysize from Crashpad.
BUG=837308
R=mark@chromium.org

Change-Id: Ibecbfc7bc2d61ee54bc1114e4b20978adbc77db2
Reviewed-on: https://chromium-review.googlesource.com/c/1393921
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
2019-01-03 19:44:15 +00:00
Tom Tan
761c6fe8be Add Windows ARM64 support to Chromium crashpad
Bug: chromium:893460
Change-Id: Ifbeb6f937a6b96c77b02dcf8afe492c5bc617435
Reviewed-on: https://chromium-review.googlesource.com/c/1347773
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2018-12-12 21:54:55 +00:00
Scott Graham
ad788280e4 fuchsia: Disable HTTPS tests to avoid (unavailable) dependency on BoringSSL
Bug: crashpad:266
Bug: fuchsia:DX-690
Change-Id: Ica6c2d6d345cbda17fd67de69e7c770389808f63
Reviewed-on: https://chromium-review.googlesource.com/c/1340743
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-11-16 21:19:24 +00:00
Scott Graham
1b4fdd0fd0 fuchsia: Re-enable HTTPS transport, but disable tests
The HTTPS tests are flaky on Fuchsia bots, so TLS transport was disabled.
However, a different CHECK fails in prod when a crash is attempted to be
uploaded via an 'https' url. So for now, re-enable the https transport,
but disable the https tests that were flaky, so they can be debugged
separately.

Additionally, there was a small error in
21edfd3c3a
that wasn't caught because these tests were disabled; fix the path to
test server certs on Fuchsia.

Bug: fuchsia:DX-382

Change-Id: I4ad0649ecb6d0644b1dfcf08bbb097d3a0cd40d0
Reviewed-on: https://chromium-review.googlesource.com/c/1265197
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Francois Rousseau <frousseau@google.com>
2018-10-05 18:16:11 +00:00
Francois Rousseau
ce122b644c [util/net] add error message to scheme check
* instead of just 'Check failed: scheme == "http"', we will have something
like 'Check failed: scheme == "http". Got 'https' for scheme in 'https://for.bar''
* clangfmt on file

Bug: fuchsia:DX-514
Change-Id: I043af7281d7f99ed5641c87920d806e340a38dea
Tested=`out/Debug/crashpad_util_test` and Fuchsia logs
Reviewed-on: https://chromium-review.googlesource.com/c/1262140
Commit-Queue: Francois Rousseau <frousseau@google.com>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2018-10-04 20:51:53 +00:00
Mark Mentovai
f8b0538406 fuchsia: Don’t require test certificate setup for disabled TLS tests
This is a follow-up to e6f26587e435.

Bug: DX-382
Change-Id: I3116ea5dd2eca33961465d62c9200aa8dd1baf5d
Reviewed-on: https://chromium-review.googlesource.com/1173339
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2018-08-13 23:35:08 +00:00
Jeremy Apthorp
f540abb506 Treat response codes in [200..203] as successful
Some crash recorders respond with non-200 2xx responses on success, e.g.
HockeyApp which responds with 202 Accepted.

Change-Id: I40de12155b44f7638a1c726090657938e3b1b557
Reviewed-on: https://chromium-review.googlesource.com/1167793
Commit-Queue: Jeremy Apthorp <jeremya@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-08-09 23:53:29 +00:00
Scott Graham
0a665e3c81 Suppress output when generating test server key
openssl outputs some useless junk when generating the test key; swallow
that.

'''
Generating a 2048 bit RSA private key
...............................................+++
...........................................+++
writing new private key to 'crashpad_util_test_key.pem'
-----
'''

Bug: crashpad:196
Change-Id: I0bdfb4f29931ef58d0c51c5e5488d3b5aeb798f0
Reviewed-on: https://chromium-review.googlesource.com/1099960
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-06-13 21:41:49 +00:00
Scott Graham
3edb7869da Roll cpp-httplib to HEAD (5b3187e2f)
Bug: crashpad:196, crashpad:227
Change-Id: If798744a71aec63e5320cc3300b70d24fed5ceae
Reviewed-on: https://chromium-review.googlesource.com/1082025
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-06-04 18:33:07 +00:00
Scott Graham
5c49c59847 fuchsia: Implement TLS support in HTTPTransportSocket
With use_boringssl_for_http_transport_socket set, this also works on
Linux, however the bots fail during run lacking libcrypto.so.1.1. So,
not enabled on Linux until that's figured out.

(Includes https://github.com/yhirose/cpp-httplib/pull/70, until it lands
and I'll do a full roll of cpp-httplib then.)

Bug: crashpad:30, crashpad:196
Change-Id: I987f6a87f8e47160c15e53fe1ce28611339069ff
Reviewed-on: https://chromium-review.googlesource.com/1075726
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-05-29 23:19:09 +00:00
Joshua Peraza
ef6d91b166 Update cpp-httplib to 37130cd
Also exclude http_transport_test_server from Android where it doesn't
build.

Change-Id: I51cc3f50e4fb9db982d91b2924b8ea87d86926d4
Reviewed-on: https://chromium-review.googlesource.com/1054160
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2018-05-11 15:32:35 +00:00
Scott Graham
f55a8d4ff3 fuchsia: Work around lack of packaging in Fuchsia tree build
Packaged test running seems to be a ways off, but with a bit of path
fiddling in test_paths.cc we can actually use the paths where the tests
are copied, so do that instead to get all the tests re-enabled. The
setup in BUILD.gn should be mostly-useful once packaging is working as
all helper/data files will need to specified there anyway.

Also, attempted fix to flaky behaviour in
ProcessReaderFuchsia.ChildThreads exposed because the tests are now
being run. zx_object_wait_many() waits on *any* of the objects, not
*all* of them. Derp!

And finally, for the same test, work around some unintuitive behaviour
in zx_task_suspend(), in particular that the thread will not be
suspended for the purpose of reading registers right away, but instead
only "sometime later", which appears in pratice to be after the next
context switch. Have ScopedTaskSuspend block for a while to try to
ensure the registers become readble, and if they don't, at least fail
noisily at that point.

Bug: crashpad:196
Change-Id: I01fb3590ede96301c941c2a88eba47fdbfe74ea7
Reviewed-on: https://chromium-review.googlesource.com/1053797
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-05-10 19:28:10 +00:00
Scott Graham
31703a585f fuchsia: When in Fuchsia tree, disable tests requiring external files
The package deployment/running is in flux at the moment. In order to get
all the other tests on to the main Fuchsia waterfall, disable the ~25
tests that require external files (for launching child processes,
loading modules, or data files) because those operations all fail on
Fuchsia-without-packages right now. Upstream this is PKG-46. Once test
packaging and running has been resolved, this can be reverted.

These tests are still run when building Crashpad standalone on Fuchsia
as the standalone build simply copies all the relevant data files to the
device in /tmp.

Bug: crashpad:196
Change-Id: I1677c394a2b9d709c59363ebeea8aff193d4c21d
Reviewed-on: https://chromium-review.googlesource.com/1045547
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2018-05-05 00:27:22 +00:00
Scott Graham
5636102fb4 Implement unlengthed response read in HTTPTransport
Of course, as soon as I tried it against the real endpoint on Fuchsia,
the server just spits out raw crash id as a string without specifying
Content-Length.

Bug: crashpad:196, crashpad:30
Change-Id: I22af87589a8801cdfece0a7b862e70e0e7097f1f
Reviewed-on: https://chromium-review.googlesource.com/1024953
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2018-04-28 00:08:11 +00:00
Scott Graham
60ae9eeadb Implementation of HTTPTransport via raw socket
Partial implementation: Currently only handles http (i.e. no TLS), only
POST, and only certain response types (only when Content-Length is
specified, and not chunked). Used for Linux and Fuchsia lacking anything
better (that's shippable). Removes libcurl HTTPTransport, since it isn't
available in the Chromium sysroot anyway.

This is an intermediate step until BoringSSL is available in the Fuchsia
SDK. Once that's available, it should be "relatively straightfoward" to
make http_transport_socket.cc secure its socket using BoringSSL or
OpenSSL depending on the platform.

Bug: crashpad:196, crashpad:227, crashpad:30
Change-Id: If33a0d3f11b9000cbc3f52f96cd024ef274a922f
Reviewed-on: https://chromium-review.googlesource.com/1022717
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2018-04-27 04:04:30 +00:00
Scott Graham
439ba730c5 Implementation in C++ of HTTPTransport test server
- Pulls in cpp-httplib for test-only usage in third_party/.
- Replaces http_transport_test_server.py with .cc server.
- Remove unnecessary Go toolchain pull. This was planned to be used for
  the test server, but the toolchain integration was too messy when
  covering all target platforms/configs.

Bug: crashpad:196, crashpad:227, crashpad:30
Change-Id: I5990781473dcadfcc036fbe711c02928638ff851
Reviewed-on: https://chromium-review.googlesource.com/1013293
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-04-20 20:56:49 +00:00
Scott Graham
eca0ea8427 Add limited version of URL cracking
This is a very basic form of URL cracking to break a
HTTPTransport::SetURL() argument up into component parts. This is split
out of the (upcoming)
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1008407
for Linux and Fuchsia.

Bug: crashpad:196
Change-Id: Iba075d9c8720c14550ce53e23d684362da84740c
Reviewed-on: https://chromium-review.googlesource.com/1010972
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-04-13 17:14:24 +00:00
Scott Graham
9affa2a0e7 Optionally stub out the libcurl-based implementation of HTTPTransport
Bug: crashpad:220
Change-Id: I6556cd5a32ea95c3dc8c5906e0857c83dc88cd9a
Reviewed-on: https://chromium-review.googlesource.com/938492
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-02-26 22:15:27 +00:00
Scott Graham
15c4fff902 Get crashpad_client_test and crashpad_handler_test building
Stubs a variety of classes (CrashReportExceptionHandler,
ExceptionHandlerServer, HTTPTransport, CrashReportDatabase).

Bug: crashpad:196
Change-Id: I4772f90d0d2ad07cc2f3c2ef119e92fde5c7acef
Reviewed-on: https://chromium-review.googlesource.com/809940
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2017-12-06 18:39:24 +00:00
Mark Mentovai
593ede52e0 Fixes for drive-by review comments after ef3ce94fbfbb
Change-Id: I28edc00549d51576ab553f401235aa1d9f669232
Reviewed-on: https://chromium-review.googlesource.com/797335
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-29 19:52:08 +00:00
Mark Mentovai
ef3ce94fbf Python 3 support for Python scripts (without compromising Python 2)
It’s better to be prepared for the future than…to not be.

This is mostly the result of running 2to3 on all .py files, with some
small shims to maintain compatibility with Python 2.
http_transport_test_server.py was slightly more involved, requiring many
objects to change from “str” to “bytes”.

The #! lines and invokers still haven’t changed, so these scripts will
still normally be interpreted by Python 2.

Change-Id: Idda3c5650f967401a5942c4d8abee86151642a2e
Reviewed-on: https://chromium-review.googlesource.com/797434
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-29 18:47:38 +00:00
Mark Mentovai
20e5aba1af URL cleanups: switch to HTTPS, fix dead ones, use canonical ones
Change-Id: I4b247d7fae1a212350f8ffcf2bf5ba1fa730f5c1
Reviewed-on: https://chromium-review.googlesource.com/780339
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-20 22:23:39 +00:00
Mark Mentovai
94a5a72efa mac: Tests that crash intentionally shouldn’t go to ReportCrash
Crashpad has many tests that crash intentionally. Some of these are
gtest death tests, and others arrange for intentional crashes to test
Crashpad’s own crash-catching logic. On macOS, all of the gtest death
tests and some of the other intentional crashes were being logged by
ReportCrash, the system’s crash reporter. Since these reports
corresponded to intentional crashes, they were never useful, and served
only to clutter ~/Library/Logs/DiagnosticReports.

Since Crashpad is adept at handling exceptions on its own, this
introduces the “exception swallowing server”,
crashpad_exception_swallower, which is a Mach exception server that
implements a no-op exception handler routine for all exceptions
received. The exception swallowing server is established as the task
handler for EXC_CRASH and EXC_CORPSE_NOTIFY exceptions during gtest
death tests invoked by {ASSERT,EXPECT}_DEATH_{CHECK,CRASH}, and for all
child processes invoked by the Multiprocess test infrastructure. The
exception swallowing server is not in effect at other times, so
unexpected crashes in test code can still be handled by ReportCrash or
another crash reporter.

With this change in place, no new reports are generated in the
user-level ~/Library/Logs/DiagnosticReports or the system’s
/Library/Logs/DiagnosticReports during a run of Crashpad’s full test
suite on macOS.

Bug: crashpad:33
Change-Id: I13891853a7e25accc30da21fa7ea8bd7d1f3bd2f
Reviewed-on: https://chromium-review.googlesource.com/777859
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-11-20 18:58:34 +00:00
Mark Mentovai
c542a5ae03 net: Remove extra spaces from WinHttp log messages
I recently (90054edf6202) removed the extra spaces from ntdll log
messages. The winhttp messages have the same problem.

Context:
https://groups.google.com/a/chromium.org/d/topic/crashpad-dev/kD3a9Jekzx0

Change-Id: I49682ba99c3dfe1cd4c49507806cf8adf154bc9f
Reviewed-on: https://chromium-review.googlesource.com/749305
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-11-01 16:48:29 +00:00
Mark Mentovai
1669ca2bac test: Rework TestPaths interface for obtaining 32-bit build artifacts
The design for running all Crashpad unit tests on Chromium’s try- and
buildbots involves pulling all tests into a single monolithic
crashpad_tests executable. Many Crashpad tests base the name of their
child executables or modules on the name of the main test executable.
Since the main test executable will have a different name in the
in-Chromium build, knowledge of the test executable name (referred to as
“module” here) needs to be added to the tests themselves.

This introduces TestPaths::BuildArtifact(), which allows the module name
to be specified. For Crashpad’s standalone build, the module name is
verified against the main test executable’s name.
TestPaths::BuildArtifact() can also locate paths in the alternate 32-bit
output directory for 64-bit Windows tests, taking on the responsibility
for what the new (5e9ed4cb9f69) TestPaths::Output32BitDirectory(), now
obsolete, did.

Bug: chromium:779790
Change-Id: I64c4a2190b6319e487c999812a7cfc512a75a700
Reviewed-on: https://chromium-review.googlesource.com/747536
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-11-01 16:44:45 +00:00
Joshua Peraza
68a0e736c6 Use a FileReaderInterface for file attachments instead of a FilePath
This is a step towards a database which gives out FileReaders in Report
objects instead of FilePaths.

Change-Id: I59704da65fc5521e5d47019416bf962c215d13bc
Reviewed-on: https://chromium-review.googlesource.com/721978
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-17 16:32:08 +00:00
Roman Margold
f3a8dbd671 net: Identify clients via URL parameters during report upload
During crash report upload, the client now provides the product
name, version, and client id via URL parameters to the crash
reporting service.
Also added percent-encoding function and a test.

Change-Id: I62f3a646d4ab6029543bd80938b79de28b1f20e4
Test: crashpad_util_test URLEncode.Empty
Test: crashpad_util_test URLEncode.ReservedCharacters
Test: crashpad_util_test URLEncode.UnreservedCharacters
Test: crashpad_util_test URLEncode.SimpleAddress
Reviewed-on: https://chromium-review.googlesource.com/493917
Commit-Queue: Roman Margold <rmargold@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-09-28 17:15:40 +00:00
Xi Cheng
01110c0a3b win: Fix %u, %d, %x/DWORD printf mismatches
To enable clang-cl's printf format string mismatch checking, a few
mismatch errors need to be fixed where DWORD (unsigned long) is printed
with %u, %d or %x (an 'l' is needed).

Change-Id: I2cbfafe823a186bfe3a555aec3a7ca03e85466f8
Reviewed-on: https://chromium-review.googlesource.com/598651
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-08-02 22:04:13 +00:00
Mark Mentovai
8f0636288a Use constexpr at namespace scope
This is essentially based on a search for “^const .*=”.

Change-Id: I9332c1f0cf7c891ba1ae373dc537f700f9a1d956
Reviewed-on: https://chromium-review.googlesource.com/585452
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-29 01:06:52 +00:00
Mark Mentovai
6dac7ecdf5 Use constexpr at function scope
This is essentially based on a search for “^ *const [^*&]*=[^(]*$”

Change-Id: Id571119d0b9a64c6f387eccd51cea7c9eb530e13
Reviewed-on: https://chromium-review.googlesource.com/585555
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-29 00:50:40 +00:00
Mark Mentovai
281be63d00 Standardize on static constexpr for arrays when possible
This uses “static” at function scope to avoid making local copies, even
in cases where the compiler can’t see that the local copy is
unnecessary. “constexpr” adds additional safety in that it prevents
global state from being initialized from any runtime dependencies, which
would be undesirable.

At namespace scope, “constexpr” is also used where appropriate.

For the most part, this was a mechanical transformation for things
matching '(^| )const [^=]*\['.

Similar transformations could be applied to non-arrays in some cases,
but there’s limited practical impact in most non-array cases relative to
arrays, there are far more use sites, and much more manual intervention
would be required.

Change-Id: I3513b739ee8b0be026f8285475cddc5f9cc81152
Reviewed-on: https://chromium-review.googlesource.com/583997
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-25 17:40:51 +00:00
Mark Mentovai
ffe4c1018c net: Update Blink source code references
The references to RFC 2388 §3 and RFC 2047 are removed. RFC 7578 has
replaced RFC 2388, and RFC 7578 acknowledges that the area of RFC 2388
called into question by the previous comment in this code was not widely
adopted. The code does not violate RFC 7578, so the TODO is removed.

Change-Id: Ie68cba49f9fbc95a4ae3a156783a6db3b406950c
Reviewed-on: https://chromium-review.googlesource.com/481244
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-19 16:08:53 +00:00
Mark Mentovai
4b450c8137 test: Use (actual, [un]expected) in gtest {ASSERT,EXPECT}_{EQ,NE}
gtest used to require (expected, actual) ordering for arguments to
EXPECT_EQ and ASSERT_EQ, and in failed test assertions would identify
each side as “expected” or “actual.” Tests in Crashpad adhered to this
traditional ordering. After a gtest change in February 2016, it is now
agnostic with respect to the order of these arguments.

This change mechanically updates all uses of these macros to (actual,
expected) by reversing them. This provides consistency with our use of
the logging CHECK_EQ and DCHECK_EQ macros, and makes for better
readability by ordinary native speakers. The rough (but working!)
conversion tool is
https://chromium-review.googlesource.com/c/466727/1/rewrite_expectassert_eq.py,
and “git cl format” cleaned up its output.

EXPECT_NE and ASSERT_NE never had a preferred ordering. gtest never made
a judgment that one side or the other needed to provide an “unexpected”
value. Consequently, some code used (unexpected, actual) while other
code used (actual, unexpected). For consistency with the new EXPECT_EQ
and ASSERT_EQ usage, as well as consistency with CHECK_NE and DCHECK_NE,
this change also updates these use sites to (actual, unexpected) where
one side can be called “unexpected” as, for example, std::string::npos
can be. Unfortunately, this portion was a manual conversion.

References:

https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#binary-comparison
77d6b17338
https://github.com/google/googletest/pull/713

Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-04 12:34:24 +00:00
Mark Mentovai
4688351623 “Promote” test::Paths::Executable() to Paths::Executable()
This supports the “double handler” or “double handler with low
probability” models from https://crashpad.chromium.org/bug/143.

For crashpad_handler to be become its own client, it needs access to its
own executable path to pass to CrashpadClient::StartHandler(). This was
formerly available in the test-only test::Paths::Executable(). Bring
that function’s implementation to the non-test Paths::Executable() in
util/misc, and rename test::Paths to test::TestPaths to avoid future
confusion.

test::TestPaths must still be used to access TestDataRoot(), which does
not make any sense to non-test code.

test::TestPaths::Executable() is retained for use by tests, which most
likely prefer the fatal semantics of that function. Paths::Executable()
is not fatal because for the purposes of implementing the double
handler, a failure to locate the executable path (which may happen on
some systems in deeply-nested directory hierarchies) shouldn’t cause the
initial crashpad_handler to abort, even if it does prevent a second
crashpad_handler from being started.

Bug: crashpad:143
Test: crashpad_util_test Paths.*, crashpad_test_test TestPaths.*
Change-Id: I9f75bf61839ce51e33c9f7c0d7031cebead6a156
Reviewed-on: https://chromium-review.googlesource.com/466346
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-04-03 18:58:01 +00:00
Mark Mentovai
8e37886d41 linux: Don’t make assumptions about the CPU revision
This code that works out the name of the CPU being built for is most
likely going to move out to be used more generally and for Android. It
should nail down the CPU name correctly when possible. Previously,
32-bit x86 always showed up as “i686” and 32-bit ARM always showed up as
“armv7l”.

Bug: crashpad:30
Change-Id: Ifd4b91f30062f5ef621a166f77a732dd8a88a58e
Reviewed-on: https://chromium-review.googlesource.com/458118
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-23 20:25:18 +00:00
Mark Mentovai
00b6442752 Make file_io reads more rational and predictable
ReadFile() attempted to continue reading after a short read. In most
cases, this is fine. However, ReadFile() would keep trying to fill a
partially-filled buffer until experiencing a 0-length read(), signaling
end-of-file. For certain weird file descriptors like terminal input, EOF
is an ephemeral condition, and attempting to read beyond EOF doesn’t
actually return 0 (EOF) provided that they remain open, it will block
waiting for more input. Consequently, ReadFile() and anything based on
ReadFile() had an undocumented and quirky interface, which was that any
short read that it returned (not an underlying short read) actually
indicated EOF.

This facet of ReadFile() was unexpected, so it’s being removed. The new
behavior is that ReadFile() will return an underlying short read. The
behavior of FileReaderInterface::Read() is updated in accordance with
this change.

Upon experiencing a short read, the caller can determine the best
action. Most callers were already prepared for this behavior. Outside of
util/file, only crashpad_database_util properly implemented EOF
detection according to previous semantics, and adapting it to new
semantics is trivial.

Callers who require an exact-length read can use the new
ReadFileExactly(), or the newly renamed LoggingReadFileExactly() or
CheckedReadFileExactly(). These functions will retry following a short
read. The renamed functions were previously called LoggingReadFile() and
CheckedReadFile(), but those names implied that they were simply
wrapping ReadFile(), which is not the case. They wrapped ReadFile() and
further, insisted on a full read. Since ReadFile()’s semantics are now
changing but these functions’ are not, they’re now even more distinct
from ReadFile(), and must be renamed to avoid confusion.

Test: *
Change-Id: I06b77e0d6ad8719bd2eb67dab93a8740542dd908
Reviewed-on: https://chromium-review.googlesource.com/456676
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-16 20:07:43 +00:00
Mark Mentovai
777e36014f linux: Add HTTPTransportLibcurl for Linux (but not Android)
BUG=crashpad:30
TEST=crashpad_util_test HTTPTransport.*

Change-Id: Ifef812830fe2d778f400467d93771dc166cef390
2017-03-03 14:47:03 -05:00
Mark Mentovai
60be5a66a0 net: Provide better HTTP User-Agent strings
Previously, macOS used “User-Agent: crashpad_util_test (unknown version)
CFNetwork/807.2.14 Darwin/16.4.0 (x86_64)” and Windows gave results like
“User-Agent: Crashpad/0.8.0”.

Now, macOS uses “User-Agent: Crashpad/0.8.0 CFNetwork/807.2.14
Darwin/16.4.0 (x86_64)” and Windows uses “User-Agent: Crashpad/0.8.0
WinHTTP/10.0.14393.351 Windows_NT/10.0.14393.0 (x64)”

Change-Id: I578b44734cf59d79e3d9b6136b4b92f05acefe71
Reviewed-on: https://chromium-review.googlesource.com/447796
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-01 17:44:08 +00:00
Mark Mentovai
1dcd45ab75 win: Pass a query component, if present, to an HTTP server
BUG=crashpad:163

Change-Id: I046dcef56a788ae04848383b0601e35d172c8183
Reviewed-on: https://chromium-review.googlesource.com/447916
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-28 19:01:34 +00:00
Mark Mentovai
0c322ecc3f Use zlib to gzip-compress uploads
This adds zlib to Crashpad. By default in standalone Crashpad builds,
the system zlib will be used where available. A copy of Chromium’s zlib
(currently a slightly patched 1.2.11) is checked out via DEPS into
third_party for use on Windows, which does not have a system zlib.

zlib is used to produce gzip streams for HTTP upload request bodies sent
by crashpad_handler by default. The Content-Encoding: gzip header is set
for these compressed request bodies. Compression can be disabled for
upload to servers without corresponding decompression support by
starting crashpad_handler with the --no-upload-gzip option.

Most minidumps compress quite well with zlib. A size reduction of 90% is
not uncommon.

BUG=crashpad:157
TEST=crashpad_util_test GzipHTTPBodyStream.*:HTTPTransport.*

Change-Id: I99b86db3952c3685cd78f5dc858a60b54399c513
Reviewed-on: https://chromium-review.googlesource.com/438585
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-02-16 16:26:19 +00:00
Mark Mentovai
1f82c6cc8a Ensure Content-Length does not appear with Transfer-Encoding
In the HTTPTransport test, verify the requirement of RFC 7230 §3.3.2
that Content-Length not appear if Transfer-Encoding is present.

TEST=crashpad_util_test HTTPTransport.*
BUG=crashpad:159

Change-Id: I51eafff9659443e1d9bb67d1213c8cecc757ded6
Reviewed-on: https://chromium-review.googlesource.com/439984
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-02-09 17:43:04 +00:00
Mark Mentovai
cd28471383 win: Implement Transfer-Encoding: chunked for HTTP requests
Chunked encoding doesn’t require the length of the request body to be
known in advance. In cases where this value isn’t independently known,
as is normal for Crashpad report uploads where the HTTP request body is
constructed on the fly, chunked encoding eliminates the need to prepare
the entire request body in memory before transmitting it. In these
cases, it’s much less wasteful.

When the length of the request body is known in advance, based on the
provision of a Content-Length header, chunked encoding is not used.
Even so, the request is sent in pieces rather than reading the entire
request into memory before sending anything.

BUG=crashpad:159
TEST=crashpad_util_test HTTPTransport.*

Change-Id: Iebb2b63b936065cb8c3c4a62b58f9c14fec43937
Reviewed-on: https://chromium-review.googlesource.com/439644
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-08 21:17:05 +00:00
Mark Mentovai
88442dd578 Merge Chromium 294442c0ce05 upstream to Crashpad
Remove stl_util from Crashpad. This also updates mini_chromium to
4f3cfc8e7c2b7d77f94f41a32c3ec84a6920f05d to remove stl_util from there
as well.

4f3cfc8e7c2b Remove stl_util from mini_chromium

BUG=chromium:555865

Change-Id: I8ecb1639a258dd233d524834ed205a4fcc641bac
Reviewed-on: https://chromium-review.googlesource.com/438865
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-02-07 21:04:42 +00:00