Comments

Request::getBody(), see #57#58

Open

JanTvrdik wants to merge 1 commit intonette:master from

JanTvrdik:request_get_body

Open

Request::getBody(), see #57 #58
JanTvrdik wants to merge 1 commit intonette:master from
JanTvrdik:request_get_body

Conversation

@JanTvrdik

Copy link

Contributor

@JanTvrdik JanTvrdik commented Feb 27, 2015

No description provided.

[画像:fprochazka]

fprochazka reviewed

Feb 27, 2015

View reviewed changes

src/Http/Request.php Outdated

Copy link

Contributor

@fprochazka fprochazka Feb 27, 2015

Choose a reason for hiding this comment

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

IMHO rawbody should return rawbody, not parsed body

Copy link

Contributor Author

@JanTvrdik JanTvrdik Feb 27, 2015

Choose a reason for hiding this comment

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

? This commit does not change getRawBody() result.

Copy link

Contributor

@fprochazka fprochazka Feb 27, 2015

Choose a reason for hiding this comment

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

You're using the same callback, that also parses the JSON for application/json, aren't you?

Copy link

Contributor

@fprochazka fprochazka Feb 27, 2015

Choose a reason for hiding this comment

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

Oh, you've added new parameter, sorry :)

@Majkl578

Copy link

Contributor

Majkl578 commented Feb 27, 2015

Although it's a nice closure magic, I'm afraid it's gonna be slow, it's not even lazy... I mean, those unnecessary body calls could be easily avoided if type check was extracted outside (like addBodyCallback('application/json', $cb)).

@JanTvrdik

Copy link

Contributor Author

JanTvrdik commented Feb 27, 2015

@Majkl578 That's how I originally wanted to do it, but then I though that it is not powerful enough. What if you want to make decision based on something different than Content-Type (e.g. HTTP method)? What if you want to handle all something/* types? That being said I like the simple solution addBodyCallback('application/json', $cb) more than the current one.

@JanTvrdik

Copy link

Contributor Author

JanTvrdik commented Apr 23, 2015 •

edited

Loading

~~(削除) Sidenote: it must somehow workaround the JSON decode DOS vulnerability. (削除ここまで)~~

@dg dg force-pushed the master branch from 78a872a to 360dadc Compare

June 19, 2015 12:10

@dg dg force-pushed the master branch 7 times, most recently from 1dc599c to f35d20f Compare

November 5, 2015 00:41

@dg dg force-pushed the master branch from 805ff25 to dadd667 Compare

February 8, 2016 13:29

@dg dg force-pushed the master branch from 51568f8 to 2381182 Compare

April 11, 2016 01:45

@dg dg force-pushed the master branch from 19c6fa9 to 28af524 Compare

April 21, 2016 11:29

@dg dg force-pushed the master branch from 6286f91 to d523f35 Compare

May 9, 2016 12:49

@JanTvrdik JanTvrdik force-pushed the request_get_body branch 2 times, most recently from f73e5f9 to 48409af Compare

June 5, 2016 07:15

@JanTvrdik JanTvrdik changed the title ~~(削除) WIP: Request::getBody(), see #57 (削除ここまで)~~ (追記) Request::getBody(), see #57 (追記ここまで)

Jun 5, 2016

@JanTvrdik

Copy link

Contributor Author

JanTvrdik commented Jun 5, 2016

I think that this is now ready to merge.

@JanTvrdik JanTvrdik force-pushed the request_get_body branch from 48409af to 48a3e5f Compare

June 5, 2016 07:17

[画像:enumag]

enumag reviewed

Jun 5, 2016

View reviewed changes

src/Http/Request.php Outdated

* @throws InvalidRequestBodyException

public function getBody()

{

Copy link

Contributor

@enumag enumag Jun 5, 2016

Choose a reason for hiding this comment

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

indentation

@JanTvrdik JanTvrdik force-pushed the request_get_body branch from 48a3e5f to 6363c41 Compare

June 5, 2016 07:27

@enumag

Copy link

Contributor

enumag commented Jun 5, 2016 •

edited

Loading

@JanTvrdik What about the JSON decode DOS vulnerability? I didn't see anything that would solve it in your code (but I might have missed it of course).

@JanTvrdik

Copy link

Contributor Author

JanTvrdik commented Jun 5, 2016 •

edited

Loading

@enumag I ignore it the same way everybody in the PHP worlds ignores it. Those few that are concern may replace the body parser for JSON with sth smarter.

@JanTvrdik


 Request: add getBody (interface BC break)

1627f10

@JanTvrdik JanTvrdik force-pushed the request_get_body branch from 6363c41 to 1627f10 Compare

June 5, 2016 12:30

@JanTvrdik

Copy link

Contributor Author

JanTvrdik commented Jun 5, 2016

BTW: PSR7 calls this method getParsedBody() instead of just getBody(). It is longer but a bit more descriptive.

[画像:dg]

dg reviewed

Jun 6, 2016

View reviewed changes

src/Http/RequestFactory.php

if ($body === NULL) {

$contentType = $request->getHeader('Content-Type');

foreach ($this->bodyParsers as $parserContentType => $parser) {

if (stripos($contentType, $parserContentType) === 0) {

Copy link

Member

@dg dg Jun 6, 2016

Choose a reason for hiding this comment

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

strcasecmp is better

Copy link

Contributor Author

@JanTvrdik JanTvrdik Jun 6, 2016

Choose a reason for hiding this comment

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

You mean strncasecmp?

Copy link

Member

@dg dg Jun 6, 2016

Choose a reason for hiding this comment

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

Yes. Its PHP shame that there is no startsWith function.

@dg dg force-pushed the master branch from 9a14e6e to a20fb8f Compare

November 14, 2023 18:31

@dg dg force-pushed the master branch 5 times, most recently from 55488bd to 2042d2e Compare

December 11, 2023 13:01

@dg dg force-pushed the master branch from 2042d2e to 3dd9381 Compare

January 30, 2024 18:25

@dg dg force-pushed the master branch 2 times, most recently from 4960652 to 5e67add Compare

May 2, 2024 10:56

@dg dg force-pushed the master branch from 5e67add to 93b51fb Compare

June 18, 2024 21:40

@dg dg force-pushed the master branch 5 times, most recently from 689f4ae to 33aae19 Compare

November 5, 2024 06:45

@dg dg force-pushed the master branch 4 times, most recently from 09923de to 02ae846 Compare

January 16, 2025 04:45

@dg dg force-pushed the master branch from 02ae846 to 7a5733c Compare

February 3, 2025 00:38

@dg dg force-pushed the master branch from 0cd27f3 to a02f3d1 Compare

June 19, 2025 17:23

@dg dg force-pushed the master branch from a02f3d1 to ee25505 Compare

October 30, 2025 22:37

@dg dg force-pushed the master branch 7 times, most recently from dc02250 to 80e8e2b Compare

December 30, 2025 00:44

@dg dg force-pushed the master branch from 80e8e2b to 423ec3d Compare

February 9, 2026 16:28

Uh oh!

Comments

Conversation

@JanTvrdik JanTvrdik commented Feb 27, 2015

Uh oh!

@fprochazka fprochazka Feb 27, 2015

Choose a reason for hiding this comment

Uh oh!

@JanTvrdik JanTvrdik Feb 27, 2015

Choose a reason for hiding this comment

Uh oh!

@fprochazka fprochazka Feb 27, 2015

Choose a reason for hiding this comment

Uh oh!

@fprochazka fprochazka Feb 27, 2015

Choose a reason for hiding this comment

Uh oh!

Majkl578 commented Feb 27, 2015

Uh oh!

JanTvrdik commented Feb 27, 2015

Uh oh!

JanTvrdik commented Apr 23, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JanTvrdik commented Jun 5, 2016

Uh oh!

@enumag enumag Jun 5, 2016

Choose a reason for hiding this comment

Uh oh!

enumag commented Jun 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JanTvrdik commented Jun 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JanTvrdik commented Jun 5, 2016

Uh oh!

@dg dg Jun 6, 2016

Choose a reason for hiding this comment

Uh oh!

@JanTvrdik JanTvrdik Jun 6, 2016

Choose a reason for hiding this comment

Uh oh!

@dg dg Jun 6, 2016

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JanTvrdik commented Apr 23, 2015 •

edited

Loading

enumag commented Jun 5, 2016 •

edited

Loading

JanTvrdik commented Jun 5, 2016 •

edited

Loading