30 Commits

Author SHA1 Message Date
Scott Graham
3d598cdbcd Change file op |ssize_t|s to FileOperationResult
R=mark@chromium.org

Review URL: https://codereview.chromium.org/1416493006 .
2015-10-22 16:14:18 -07:00
Mark Mentovai
9fdb70738b mac: 10.11 SDK compatibility
This doesn’t really provide compatibility, it just ignores the
deprecation warning for +[NSURLConnection
sendSynchronousRequest:returningResponse:error:].

The suggested replacement, NSURLSession, was new in 10.9, and this code
needs to run on 10.6, so it’s not usable here, at least not without a
runtime check.

BUG=crashpad:65
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1395673002 .
2015-10-07 16:16:53 -04:00
Scott Graham
0b022d72a2 Include implicit_cast.h at all users of it.
The implicit_cast in base will be no more, make sure we have a reference
to the crashpad version at all callsites.

BUG=529769, 472900, crashpad:51
R=mark@chromium.org, scottmg@chromium.org

Review URL: https://codereview.chromium.org/1344683002 .
2015-09-14 14:51:05 -07:00
Scott Graham
ccf9f98519 win xp: Don't use ICU_REJECT_USERPWD with WinHttpCrackUrl
Fails on XP with ERROR_INVALID_PARAMETER (undocumented). Not overly
important that we reject embedded user/passwords in the URL for this
use case.

R=mark@chromium.org
BUG=crashpad:50

Review URL: https://codereview.chromium.org/1339793002 .
2015-09-11 13:23:59 -07:00
Scott Graham
78bba8808b win: Pass WINHTTP_FLAG_SECURE when necessary
Otherwise the server drops us when connecting to an https endpoint,
and WinHttpReceiveResponse fails with an obscure error.

R=mark@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/1317023003 .
2015-08-31 13:29:00 -07:00
Mark Mentovai
14a2241274 HTTPTransport test: Deal with limited-size pipe buffers
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 .
2015-08-18 17:52:12 -04:00
Mark Mentovai
f496130fd5 HTTPTransportMac: CFStream Read() must always set at_eof
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 .
2015-08-18 15:42:34 -04:00
Mark Mentovai
809ea8158d test: Move util/test to its own top-level directory, test.
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
2015-03-31 17:44:14 -04:00
Mark Mentovai
d5ddd14ee1 Improve map insertion operations.
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
2015-03-31 14:29:32 -04:00
Robert Sesek
f0ee5f0efe Remove NSInputStream used in HTTPTransportMac and use a CFReadStream instead.
NSInputStream requires overriding and implementing private methods in order to
use it with NSURLConnection [1]. It is cleaner to use the private but stable
and open source CFStreamAbstract.h header from CF-Lite to implement a
CFReadStream. Since CFReadStream is toll-free bridged to NSInputStream, the
remainder of the HTTPTransport code can remain unchanged.

[1] http://lists.apple.com/archives/macnetworkprog/2007/May/msg00055.html

BUG=crashpad:15
R=mark@chromium.org

Review URL: https://codereview.chromium.org/993413003
2015-03-11 16:59:59 -04:00
Mark Mentovai
26804a0be1 Add ASSERT_DEATH_CHECK() to do ASSERT_DEATH() of CHECK() failures.
Likewise for EXPECT_DEATH_CHECK() and EXPECT_DEATH().

In the in-Chromium build configured for official builds in Release mode,
CHECK() throws away its condition string and stream parameters without
ever printing them, although it still evaluates the condition and
triggers death appropriately. {ASSERT,EXPECT}_DEATH(statement, regex)
will not work correctly for any regex that attempts to match what
CHECK() prints. In these build configurations,
{ASSERT,EXPECT}_DEATH_CHECK() use a match-all regex (""). In other build
configurations, they transparently wrap {ASSERT,EXPECT}_DEATH().

BUG=crashpad:12
R=rsesek@chromium.org, scottmg@chromium.org

Review URL: https://codereview.chromium.org/992693003
2015-03-09 18:02:14 -04:00
Mark Mentovai
32a9d410ca Locate test data more robustly.
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
2015-03-09 15:13:13 -04:00
Scott Graham
07fcf63c21 win: fixes for Windows x64
Mostly size_t <-> unsigned int warnings, but I also had a mistake in
PROCESS_BASIC_INFORMATION, the pids are 32-on-32 and 64-on-64.

The Windows build is still x86 until https://codereview.chromium.org/981333002/.
I don't think I'll bother maintaining the x86 build for now, though we will probably
need it for x86 OSs in the future. It should be straightforward to revive it once we
need it, and have bots to support it.

R=mark@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/983103004
2015-03-06 16:05:34 -08:00
Scott Graham
b16b89c89d Make HTTPTransportWin respect user timeout
Uses solution suggested in linked bug. No test as it'd be flaky, slow,
or both.

R=mark@chromium.org
BUG=crashpad:8

Review URL: https://codereview.chromium.org/897393002
2015-02-06 10:10:55 -08:00
Mark Mentovai
242619d958 HTTPTransport: callers should be able to obtain the HTTP response body.
This adds a new optional out-parameter to
HTTPTransport::ExecuteSynchronously() and provides Mac and Windows
implementations.

BUG=crashpad:5
R=rsesek@chromium.org, scottmg@chromium.org

Review URL: https://codereview.chromium.org/885183004
2015-02-05 18:05:40 -05:00
Scott Graham
892c29e8ba Reorganize Multiprocess and implement for Windows
- 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
2015-01-28 14:49:42 -08:00
Scott Graham
7c9bd944ae win: Add implementation of HTTPTransport based on WinHTTP
(There's also https://codereview.chromium.org/854363006/ based on
WinInet, I'm still a little uncertain which is preferable here.)

R=cpu@chromium.org, mark@chromium.org, rsesek@chromium.org, ananta@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/852213004
2015-01-26 13:31:35 -08:00
Scott Graham
9b05b910d4 win: FILE_PATH_LITERALs in http_multipart_builder_test.cc
clang-format wanted to rewrap this way.

R=mark@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/814683003
2015-01-07 15:07:40 -08:00
Scott Graham
119c4fdd93 win: various porting for http_body_test.cc
- Wrap constants in FILE_PATH_LITERAL for L"".
- dynamic allocation, as VS otherwise complains about lack of constant
expression:

d:\src\crashpad\crashpad\util\net\http_body_test.cc(182) : error C2057: expected constant expression
d:\src\crashpad\crashpad\util\net\http_body_test.cc(182) : error C2466: cannot allocate an array of constant size 0
d:\src\crashpad\crashpad\util\net\http_body_test.cc(182) : error C2133: 'buf' : unknown size
d:\src\crashpad\crashpad\util\net\http_body_test.cc(183) : error C2070: 'uint8_t []': illegal sizeof operand
d:\src\crashpad\crashpad\util\net\http_body_test.cc(196) : error C2070: 'uint8_t []': illegal sizeof operand

R=mark@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/837293002
2015-01-07 15:05:38 -08:00
Scott Graham
9cfd2c515e Make http_body cross-platform
'util' builds on Windows after this.

R=mark@chromium.org, rsesek@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/791493007
2014-12-19 15:21:19 -08:00
Scott Graham
10165ce449 Cross platform low level file IO wrappers
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
2014-12-17 14:35:18 -08:00
Scott Graham
3eeae10ebe win: avoid warning on return of base::RandGenerator
Else,

d:\src\crashpad\crashpad\util\net\http_multipart_builder.cc(50) : warning C4244: 'initializing' : conversion from 'uint64_t' to 'int', possible loss of data

R=mark@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/796643006
2014-12-17 10:50:05 -08:00
Mark Mentovai
22cf9e28d5 util_test: Don’t crash when run from the wrong location.
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
2014-12-15 16:40:16 -05:00
Robert Sesek
e5048b3a80 In CompositeHTTPBodyStream, coalesce small GetBytesBuffer()s to better fill the buffer.
R=mark@chromium.org

Review URL: https://codereview.chromium.org/707223002
2014-11-07 12:08:14 -05:00
Mark Mentovai
48b1964d1b Use implicit_cast<> instead of static_cast<> whenever possible.
implicit_cast<> only performs a cast in cases where an implicit
conversion would be possible. It’s even safer than static_cast<> It’s an
“explicit implicit” cast, which is not normally necsesary, but is
frequently required when working with the ?: operator, functions like
std::min() and std::max(), and logging and testing macros.

The public style guide does not mention implicit_cast<> only because it
is not part of the standard library, but would otherwise require it in
these situations. Since base does provide implicit_cast<>, it should be
used whenever possible.

The only uses of static_cast<> not converted to implicit_cast<> are
those that require static_cast<>, such as those that assign an integer
constant to a variable of an enum type.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/700383007
2014-11-06 16:44:38 -05:00
Mark Mentovai
486429e4f4 util/net: Place death tests into a *DeathTest test case.
Not doing this causes gtest to issue these warnings:

[WARNING] ../../third_party/gtest/gtest/src/gtest-death-test.cc:825::
Death tests use fork(), which is unsafe particularly in a threaded
context. For this test, Google Test detected 4 threads.

The gtest documentation recommends giving the test case a name ending in
DeathTest. Test cases named according to this convention run before all
other tests. Other death tests in Crashpad also follow this convention.

https://code.google.com/p/googletest/wiki/AdvancedGuide#Death_Tests_And_Threads

TEST=util_test HTTPMultipartBuilderDeathTest.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/694963002
2014-10-31 15:39:16 -04:00
Robert Sesek
d88711adfa Add HTTPTransport, a Mac implementation, and an end-to-end test.
BUG=https://crbug.com/415544
R=mark@chromium.org

Review URL: https://codereview.chromium.org/692963002
2014-10-31 12:17:32 -04:00
Robert Sesek
123e01f96d Treat '+' as a safe MIME type character.
This addresses a review comment from
https://codereview.chromium.org/681303003/diff/120001/util/net/http_multipart_builder.cc#newcode107

R=mark@chromium.org

Review URL: https://codereview.chromium.org/694483002
2014-10-30 09:22:39 -04:00
Robert Sesek
9db5d6f773 Add HTTPMultipartBuilder and its test.
BUG=https://crbug.com/415544
R=mark@chromium.org

Review URL: https://codereview.chromium.org/681303003
2014-10-29 19:13:24 -04:00
Robert Sesek
977a7a8052 Add HTTPBodyStream interface, three concrete implementations, and their tests.
BUG=415544
R=mark@chromium.org

Review URL: https://codereview.chromium.org/669153006
2014-10-24 15:04:25 -04:00