Getting a request's body seems overly complex

@igorw

So just been in chat asking about getting the request body. I just think it could be improved and this seems like a better forum, since it's got longer text.

@igorw and @cboden pointed out that it's meant to be streamed/piped. And were helpful in pointing me in the direction of the information to get the request body. I can achieve getting the body data in multiple ways, but in each case it seems that I'm getting the data after the request has finished. This could just be me doing it wrong. My latest trial can be found at https://gist.github.com/icambridge/d8c1ea63cce6733bc5d5 I did also manage to get the data using the on data event and again this was executed after the request was over.

I think personally that the body should be an attribute in request object as it clearly belongs to that request. I also think it should be easy to fetch. I do understand the desire to keep memory low when you don't know what size the body will be, however in some cases you have a rough idea and that it could be small. So my idea is to refactor the server class so you can change RequestHeaderParser if you desire to so it returns a BodiedRequest object so if you want it to be a simple getBody you can do so. I'm willing to do the leg work I just don't want to spend time doing then have some point out all the things wrong with that approach.

This is quite straight-forward with BufferedSink:

BufferedSink::createPromise($request)->then(function ($body) {
    ...
});

Not holding the entire request body in memory is a fundamental design decision. What will you do when the request is 4MB of POST data? Or a 3GB file upload? The current design will always give you a stream and allow you to process the body in a streaming manner.

While that is relatively straightforward. For something as simple as getting a bit of data relating to the request it is a lot more complex than just getBody. You've got streams and then promises all in the mix.

I get and understand the point of streaming the request body for the situations you've mentioned. I think I've poorly described what I want to do. Instead of changing the default behaviour I want to be able to choose to change that behaviour if I decide. So the default would remain streaming however I could inject a different RequestHeaderParser object that can behave in the manner I would like.

So basically create a RequestHeaderParserInterface and add an optional parameter to the server constructor. If it's empty use the default parser else use the parser provided. That way I could in my code change how that works without affecting how the lib itself works.

(Also random note I've also noticed the error in my example code which was making me so confused I was using end instead of write on the response in the request handler. So when I was trying to use it in the promise callback it couldn't write because the response had ended.)

A quick example of what I mean can be found at https://github.com/icambridge/http/commit/e086ce505a675b0eb06b2c1e3c68ecbfadcbfb25

@cboden

I'm not sure if I understand this completely.
It seems that I have to end() my $response before I can process the data on my request? Is that it?

How do I make a request like this:

curl -X POST http://myserver -d "cboden"

respond with:

Hello cboden!

?

@cboden @icambridge

@naroga

@naroga No I believe you can process it without ending the request, once you end the request as far as I understand you can't send any more data.

@icambridge

@icambridge please forgive me for not understanding, but can you please provide a simple snippet that retrieves the content payload (the raw request body) without ending the request?

@icambridge

If I try the following provided snippet:

BufferedSink::createPromise($request)->then(function ($body) use (&$response) {
    $response->writeHead(...);
    $response->end("Hello . " $body);
});

And try the following request

curl -X POST http://myhost -d 'icambridge'

It will not respond with Hello icambridge as expected. It will just hold the request until curl times out.

However, if I end the request:

BufferedSink::createPromise($request)->then(function ($body) use (&$response) {
    echo $body;
});

$response->writeHead(....);
$response->end("Hello world");

It will always respond to the browser with 'Hello world', but will print 'icambridge' in my server console.

I want to respond to the browser with Hello icambridge. How do I do that?

All in all, I'm sure I'll have to agree with @icambridge. Streamed requests are very important for requests with huge payloads, but I feel like the default way to retrieve the raw body should be much simpler. If you check #30, there is someone else who couldn't figure out how to get the payload. Leave it to the to user to decide if he wants to retrieve it at once or in a streamed fashion.

BufferedSink::createPromise($request)->then(function ($body) use ($response) {
    $response->write("Hello " . $body);
});

$response->writeHead(....);

Should work for you.

@icambridge

@icambridge nope. This snippet times out all my requests. If don't explicitly end() the $response, it doesn't print anything and the browser times out after a minute or so.

So use end at some point?

BufferedSink::createPromise($request)->then(function ($body) use ($response) {
    $response->write("Hello " . $body);
    //IF I PUT END HERE, IT NEVER ENDS.
});

$response->writeHead(....);
//IF I PUT END HERE, IT NEVER WRITES 'HELLO $BODY'

Merge pull request #13 from onigoetz/master

This is no longer an issue, since pull request #13 has been merged (b36fb25).

Sorry about the repeated comments, but this discussion was kind of scattered all around, and I figured it'd be best to comment everywhere just in case future people have the same issues or don't know how to proceed.

Navigation Menu

Repository navigation

Getting a request's body seems overly complex #7

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Getting a request's body seems overly complex #7

Description

Activity

igorw commented on Jun 17, 2014

icambridge commented on Jun 18, 2014

icambridge commented on Jun 18, 2014

naroga commented on Aug 10, 2015

icambridge commented on Aug 10, 2015

naroga commented on Aug 10, 2015

naroga commented on Aug 10, 2015

icambridge commented on Aug 10, 2015

naroga commented on Aug 10, 2015

icambridge commented on Aug 10, 2015

naroga commented on Aug 10, 2015

naroga commented on Aug 10, 2015

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions