mirror of
https://github.com/chromium/crashpad.git
synced 2024-12-27 15:32:10 +08:00
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 .
This commit is contained in:
parent
a691448ffb
commit
f496130fd5
@ -87,15 +87,17 @@ class HTTPBodyStreamCFReadStream {
|
||||
CFStreamError* error,
|
||||
Boolean* at_eof,
|
||||
void* info) {
|
||||
if (buffer_length == 0)
|
||||
if (buffer_length == 0) {
|
||||
*at_eof = FALSE;
|
||||
return 0;
|
||||
}
|
||||
|
||||
ssize_t bytes_read = GetStream(info)->GetBytesBuffer(buffer, buffer_length);
|
||||
if (bytes_read == 0) {
|
||||
*at_eof = TRUE;
|
||||
} else if (bytes_read < 0) {
|
||||
if (bytes_read < 0) {
|
||||
error->error = -1;
|
||||
error->domain = kCFStreamErrorDomainCustom;
|
||||
} else {
|
||||
*at_eof = bytes_read == 0;
|
||||
}
|
||||
|
||||
return bytes_read;
|
||||
|
@ -278,6 +278,28 @@ TEST(HTTPTransport, UnchunkedPlainText) {
|
||||
test.Run();
|
||||
}
|
||||
|
||||
TEST(HTTPTransport, Upload33k) {
|
||||
// On OS X, NSMutableURLRequest winds up calling into a CFReadStream’s Read()
|
||||
// callback with a 32kB buffer. Make sure that it’s able to get everything
|
||||
// when enough is available to fill this buffer, requiring more than one
|
||||
// Read().
|
||||
|
||||
std::string request_string(33 * 1024, 'a');
|
||||
scoped_ptr<HTTPBodyStream> body_stream(
|
||||
new StringHTTPBodyStream(request_string));
|
||||
|
||||
HTTPHeaders headers;
|
||||
headers[kContentType] = "application/octet-stream";
|
||||
headers[kContentLength] =
|
||||
base::StringPrintf("%" PRIuS, request_string.size());
|
||||
HTTPTransportTestFixture test(headers, body_stream.Pass(), 200,
|
||||
[](HTTPTransportTestFixture* fixture, const std::string& request) {
|
||||
size_t body_start = request.rfind("\r\n");
|
||||
EXPECT_EQ(33 * 1024u + 2, request.size() - body_start);
|
||||
});
|
||||
test.Run();
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace test
|
||||
} // namespace crashpad
|
||||
|
Loading…
x
Reference in New Issue
Block a user