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>
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>
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>
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>
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>
- 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>
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>
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>
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-comparison77d6b17338https://github.com/google/googletest/pull/713
Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
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>
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>
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>
Use “macOS” as the generic unversioned name of the operating system in
comments. For version-specific references, use Mac OS X through 10.6, OS
X from 10.7 through 10.11, and macOS for 10.12.
Change-Id: I1ebee64fbf79200bc799d4a351725dd73257b54d
Reviewed-on: https://chromium-review.googlesource.com/408269
Reviewed-by: Robert Sesek <rsesek@chromium.org>
This was done in Chromium’s local copy of Crashpad in 562827afb599. This
change is similar to that one, except more care was taken to avoid
including headers from a .cc or _test.cc when already included by the
associated .h. Rather than using <stddef.h> for size_t, Crashpad has
always used <sys/types.h>, so that’s used here as well.
This updates mini_chromium to 8a2363f486e3a0dc562a68884832d06d28d38dcc,
which removes base/basictypes.h.
e128dcf10122 Remove base/move.h; use std::move() instead of Pass()
8a2363f486e3 Move basictypes.h to macros.h
R=avi@chromium.org
Review URL: https://codereview.chromium.org/1566713002 .
This more-natural spelling doesn’t require Crashpad developers to have
to remember anything special when writing code in Crashpad. It’s easier
to grep for and it’s easier to remove the “compat” part when pre-C++11
libraries are no longer relevant.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1513573005 .
This unifies several things that used a 16-character random string, and
a few other users of random identifiers where it also made sense to use
a 16-character random string.
TEST=crashpad_util_test RandomString.RandomString
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1451793002 .
HTTPTransport.Upload33k failed on Windows due to WinHTTP timing out. The
test server, http_transport_test_server.py, writes the entire request to
a stdout pipe, to be received by crashpad_util_test. crashpad_util_test
is also the HTTP client, and it does not attempt to read from this pipe
until the HTTP transaction is complete. http_transport_test_server.py
must not write to stdout until the transaction is complete, otherwise,
there is a risk of deadlock if the pipe buffer fills up. The new
Upload33k test sends a large request, which was filling up the pipe
buffer on Windows.
This also adds an Upload33k_LengthUnknown test variant to exercise a
large POST when the length is not known ahead of time. This more closely
matches how Crashpad crash uploads are done on OS X.
TEST=crashpad_util_test HTTPTransport.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1286173007 .
CFStream’s CFReadStreamGetBuffer() calls the Read() callback without
initializing at_eof. The callback function is responsible for setting it
on any successful read operation. See 10.10.2 CF-1152.14/CFStream.c.
By chance, at_eof seems to always have an initial value of false on
x86_64, but true on 32-bit x86. Crashpad’s Read() callback assumed that
the initial value was always false. The discrepancy caused truncation
and possibly hangs when a 32-bit process attempted to upload a request
body larger than 32kB, the buffer size used by NSMutableURLRequest or
something between it and CFReadStream.
A new test with more than 32kB of data is added.
As discussed in:
https://groups.google.com/a/chromium.org/d/topic/crashpad-dev/Vz--qMZJRPU
TEST=crashpad_util_test HTTPTransport.Upload33k
BUG=
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1304433004 .
After 9e79ea1da719, it no longer makes sense for crashpad_util_test_lib
to “hide” in util/util_test.gyp. All of util/test is moved to its own
top-level directory, test, which all other test code is allowed to
depend on. test, too, is allowed to depend on all other non-test code.
In a future change, when crashpad_util_test_lib gains a dependency on
crashpad_client, it won’t look so weird for something in util (even
though it’s in util/test) to depend on something in client, because the
thing that needs to depend on client will live in test, not util.
BUG=crashpad:33
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1051533002
Add MapInsertOrReplace<>() to insert a key-value pair into a map if the
key is not already present, or replace the existing value for key if the
key is present. The original value can optionally be returned to the
caller in this case.
Map insertions now use either MapInsertOrReplace<>() or
std::map<>::insert() directly.
Use MapInsertOrReplace<>() when the map should be updated to contain a
mapping from a key to a value regardless of whether the key is already
present.
Use std::map<>::insert() to insert a mapping from a key to a value
without replacing any existing mapping from a key, if present. If it is
important to know whether an existing mapping from a key was present,
use the returned std::pair<>.second. If it is important to know the
existing value, use the returned std::pair<>.first->second.
This change has a slight positive impact on performance.
TEST=crashpad_util_test MapInsert.MapInsertOrReplace and others
BUG=
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1044273002
Test code that requires test data should call Paths::TestDataRoot() to
obtain the test data root. This will use the CRASHPAD_TEST_DATA_ROOT
environment variable if set. Otherwise, it will look for test data at
known locations relative to the executable path. If the test data is not
found in any of these locations, it falls back to using the working
directory, the same as the current behavior.
BUG=crashpad:4
TEST=crashpad_util_test Paths.TestDataRoot and others
R=rsesek@chromium.org, scottmg@chromium.org
Review URL: https://codereview.chromium.org/992503002
- Various "FD" to "Handle"
- Existing Multiprocess implementation moves to _posix.
- Stub implementation for _win.
At the moment, multiprocess_exec_win.cc contains implementations of both
Multiprocess methods and MultiprocessExec functions. This will need more
work in the future, but reflects the idea that all tests should be in
terms of MultiprocessExec eventually.
Currently, this works sufficiently to have util_test succeed (including
multiprocess_exec_test, and the recently ported HTTPTransport tests.)
R=mark@chromium.org
BUG=crashpad:1, crashpad:7
Review URL: https://codereview.chromium.org/880763002
Rename fd_io to file_io, and ReadFD to ReadFile, etc.
file_io.cc is the higher level versions that call the basic ReadFile/WriteFile
and then file_io_posix.cc and file_io_win.cc are the implementations of
those functions.
The Windows path is as yet untested, lacking the ability to link the test binary.
R=cpu@chromium.org, mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/811823003
When run from the wrong location and test data or other test programs
can’t be found, the tests should fail with gtest assertions. The test
executable should not crash.
BUG=crashpad:4
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/799083003
