Skip to content

Remove 'Transfer-Encoding' before emitting to the request event - #137

Merged
WyriHaximus merged 3 commits into
reactphp:masterfrom
legionth:remove-header-for-request
Mar 7, 2017
Merged

Remove 'Transfer-Encoding' before emitting to the request event#137
WyriHaximus merged 3 commits into
reactphp:masterfrom
legionth:remove-header-for-request

Conversation

@legionth

@legionthlegionth commented Mar 3, 2017

Copy link
Copy Markdown
Contributor

Currently the complete header will be emitted to the request event. The request event will always receive decoded data. So at this point there is no need to keep the header lines.

Builds on top of #116

Comment threadsrc/Server.php Outdated
$stream = new LengthLimitedStream($stream, $contentLength);
}

$request = $request->withoutHeader('Content-Length');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point in always removing this? Isn't this something (for example) a middleware might be interested in?

Also, I don't see this documented anywhere, in particular not the WHY?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some lines to the README and changed the code. The 'Content-Length' will now only be removed if also 'Transfer-Encoding' is given.

@legionthlegionth changed the title Remove 'Transfer-Encoding' and 'Content-Length' header before emitting the request eventRemove 'Transfer-Encoding' before emitting to the request eventMar 3, 2017
@clueclue added this to the v0.6.0 milestone Mar 3, 2017
clue
clue approved these changes Mar 3, 2017
@clue

clue commented Mar 3, 2017

Copy link
Copy Markdown
Member

Makes perfect sense now 👍

Comment threadREADME.md Outdated

The `Request` class is responsible for streaming the incoming request body
and contains meta data which was parsed from the request headers.
If the request body is chunked-encoded, the data will be decoded an emitted on the data event.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: and emitted

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eyes 👍 , fixed it.

@legionth
legionthforce-pushed the remove-header-for-request branch from 3f1c927 to bcfbce1CompareMarch 4, 2017 20:53
clue
clue approved these changes Mar 6, 2017
jsor
jsor approved these changes Mar 6, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@legionth@clue@jsor@WyriHaximus