Can't upload more than 32k

34 views
Skip to first unread message

Mihnea Craciun

unread,
Aug 11, 2015, 11:38:08 AM8/11/15
to Crashpad-dev
Hi guys,

I'm working to replace Breakpad with crashpad in some Mac projects I'm involved with.
It's all fine and dandy and first let me congratulate you on a very good job you've done with this new library.
I have on big issue though, I can't seem to upload more than 32k to my web server.
I was wondering if I'm doing something wrong or you have the same issue.
The problem is I cannot get it to upload more than 32k only with the new "setHTTPBodyStream".
This is a code snipped that I had to change to get it upload the whole dump file:

 /*HTTPBodyStreamCFReadStream body_stream_cf(body_stream());

    base::scoped_nsobject<NSInputStream> input_stream(

        body_stream_cf.CreateInputStream());

    [request setHTTPBodyStream:input_stream.get()];*/

    NSData* data = [NSData dataWithContentsOfFilebase::SysUTF8ToNSString("/Users/mc/Library/Caches/TestCrashpad/completed/5c827674-2e2a-486e-a8b4-e406b142b935.dmp")];

    [request setHTTPBody:data];


To me it looks like the combination between sendSynchronousRequest and NSInputStream(which I believe needs a run loop)  doesn't work.

But I'm very far from being a Mac expert, so here I am, asking you guys if you have the same issue as I do.


Thank you very much in advance,

Mihnea Craciun


Robert Sesek

unread,
Aug 17, 2015, 5:02:54 PM8/17/15
to Mihnea Craciun, Crashpad-dev
Hi Mihnea,

Sorry for the delay in responding; I was at a conference last week.

I do not believe there is an issue with Crashpad. I wrote this small test to verify:

(create the temp file with 33k of 'a'):
$ python -c 'print "a" * (33 * 1024)' > /tmp/33k.txt

(add this to util/net/http_transport_test.cc):
TEST(HTTPTransport, Upload33k) {
  scoped_ptr<HTTPBodyStream> body_stream(new FileHTTPBodyStream(base::FilePath("/tmp/33k.txt")));

  HTTPHeaders headers;
  headers[kContentType] = "application/octet-stream";
  headers[kContentLength] = "33793";   // optional
  HTTPTransportTestFixture test(headers, body_stream.Pass(), 200,
      [](HTTPTransportTestFixture* fixture, const std::string& request) {
        size_t body_start = request.rfind("\r\n");
        LOG(INFO) << "body length = " << request.size() - body_start;
        LOG(INFO) << request;
      });
  test.Run();
}

… and 33k of the letter 'a' were transmitted successfully. I ran this test on 10.9.5. My guess is that either 1) you are running on an OS that has different CFNetwork behavior, or 2) your webserver is ending the connection after 32k.

No manual run loop is necessary, since the HTTP request is executed synchronously. CFNetwork creates its own thread to manage the run loop.

- Robert


rsesek / @chromium.org

--
You received this message because you are subscribed to the Google Groups "Crashpad-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to crashpad-dev...@chromium.org.
To post to this group, send email to crashp...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/crashpad-dev/d9608eee-ac3c-46b3-b7f7-093a4100d76b%40chromium.org.

mih...@arkaos.net

unread,
Aug 18, 2015, 1:00:18 AM8/18/15
to Crashpad-dev, mcra...@gmail.com
Hi Robert,

Thank you very much for your reply and for taking the time to look into it.
For now I am happy with the patch I've made which works for me.
As far as the test code you wrote goes, while it seems similar, it doesn't test the real scenario.
The difference being the transfer encoding chosen for the HTTP connection. Let me explain: in the library code, the total content length of the request is not known(computed) before hand, hence the request is sent out with chunked transfer encoding. In the test you wrote, the content length is known and set on the request, thus eliminating the need for transfer encoding. This is one important difference I can spot right away. And it's not about the format of the HTTP request on the wire, but most likely about different logic the HTTP library has to employ to extract data from the caller(multiple reads from the input stream until EOF condition is signaled, probably by a 0 byte successful read), which didn't happen: I could verify only one read was issued on the input stream, with a 32k data buffer.

As far as my webserver rejecting the request, I was able to verify with a network sniffer that it wasn't the case. It was actually the client who sent the reset packet at some point without following any obvious logic. It wasn't the connection timeout interval, since that most likely would have resulted in an error being returned from the ExecuteSynchronously method. But no, the ExecuteSynchronously method would not signal anything wrong.

I've done all the testing I mentioned above on 10.10.4, so I doubt that makes a difference.

Hope this helps reproducing the issue on your end.

Best regards,
Mihnea

Mark Mentovai

unread,
Aug 18, 2015, 9:47:13 AM8/18/15
to mih...@arkaos.net, Crashpad-dev
We use this code with files and requests larger than 32kB all the time, and we haven’t seen anything like what you describe. When Crashpad reads a file, it doesn’t consider the on-disk length at all. It reads until it can’t read any more.

Can you express the problem you’re seeing in a failing test in the same manner that Robert provided a test that passes?

I also want to call your attention to the example you gave in your initial message to this thread. It doesn’t show that you’re making an equivalent request to how we use this code. Crashpad calls -[NSMutableURLRequest setHTTPBodyStream:] and passes a stream that contains a request body in multipart/form-data format. One of the parts of this request body will be a minidump file. The body stream is created by HTTPMultipartBuilder. You show a call to -[NSMutableURLRequest setHTTPBody:] where the body is not multipart/form-data, but simply the contents of a minidump file.

mih...@arkaos.net

unread,
Aug 18, 2015, 12:39:38 PM8/18/15
to Crashpad-dev, mih...@arkaos.net
Hi Mark,

I thought it will be hard to create a test case to show off the problem, until I actually inspected the test framework and saw that you guys did a tremendous job of creating a very powerful and seemingly complete test harness. Well done!

On the other hand, I only needed to change Robert's test just a bit to surface the issue.
Here is my test case, which , as expected, fails.
Also, I noticed that, although the request is presented to the server as having a transfer-encoding of type chunked(probably by NSURLRequest object), its body format (as reflected back from the python server) doesn't follow the chunked encoding - it lacks the chunk headers. It's probably expected that the data served up via the NSInputStream is already properly formatted for chunked transfer encoding. But that can be easily fixed.

Hope this makes tracking the issue easier.

scoped_ptr<HTTPBodyStream> body_stream(new FileHTTPBodyStream(base::FilePath("/tmp/33k.txt")));

HTTPHeaders headers;
headers[kContentType] = "application/octet-stream";
HTTPTransportTestFixture test(headers, body_stream.Pass(), 200,
[](HTTPTransportTestFixture* fixture, const std::string& request) {
size_t body_start = request.rfind("\r\n");
body_start += sizeof("\r\n");
LOG(INFO) << "body length = " << request.size() - body_start;
LOG(INFO) << request;
EXPECT_GT(request.size() - body_start, 32u * 1024u);
});
test.Run();

Cheers,
Mihnea

Robert Sesek

unread,
Aug 18, 2015, 1:00:33 PM8/18/15
to mih...@arkaos.net, Crashpad-dev
Hi Mihnea,

In my test, I marked the line |headers[kContentLength] = "33793";| as optional and can be removed. The test passes for me on both 10.9.5 and 10.10.4.

I can also add this line in my test and it passes:

  EXPECT_EQ(33u * 1024, request.size() - body_start - 3);  // -3 for \r\n between headers and body, and \n at the end.

Regarding the chunked encoding, you're mixing up your layers a little. The HTTPBodyStream passed to HTTPTransport should not be chunked; this is the raw data to upload. The transport, via NSURLRequest, will automatically chunk the entire data stream if the Content-Length header is not present. When received by Python test server, which also implements a chunked transfer decoder, and the body will properly un-chunked and communicated back to the test harness. This ensures proper end-to-end testing of chunked transfer encoding.

I'm not quite sure where your problem would exist, if the test you provided is failing.

- Robert

rsesek / @chromium.org

mih...@arkaos.net

unread,
Aug 18, 2015, 2:13:04 PM8/18/15
to Crashpad-dev, mih...@arkaos.net
Hi Robert,

Thanks for your quick reply and for your explanation.
I didn't follow the test deep enough to figure out the python script actually preprocesses the request before feeding it back.

I did however found out one extra vital condition for issue to appear.
It only malfunctions in the 32 bit builds.

I'm building the crashpad library as follows:

~/depot_tools/gclient config --unmanaged 'https://chromium.googlesource.com/crashpad/crashpad'
~/depot_tools/gclient sync --nohooks
cd crashpad
python build/gyp_crashpad.py -Dtarget_arch=ia32
~/depot_tools/ninja -C out/Debug
python build/run_tests.py Debug

For me, in this configuration, the test 33k test I've sent earlier fails.

It's obviously not a big issue because there's very little need for the library and the accompanying tools to be built in 32 bits, but at least now I know why it failed for me and not for the rest of the world.

Thank you again for taking the time to help me figure this out.

Cheers,
Mihnea

Mark Mentovai

unread,
Aug 18, 2015, 3:22:10 PM8/18/15
to Mihnea Craciun, Crashpad-dev

Thanks for your quick reply and for your explanation.
I didn't follow the test deep enough to figure out the python script actually preprocesses the request before feeding it back.

I did however found out one extra vital condition for issue to appear.
It only malfunctions in the 32 bit builds.

That’s the smoking gun we needed to reproduce the problem you’re seeing.

I figured out the bug and came up with a fix: https://codereview.chromium.org/1304433004/.

Thanks for the report, and for insisting that there really was a problem despite our telling you that there wasn’t.
Reply all
Reply to author
Forward
0 new messages