Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

#63 no json_decode() call if request body is empty #64

Merged
merged 3 commits into from Jul 25, 2018
Merged

#63 no json_decode() call if request body is empty #64

merged 3 commits into from Jul 25, 2018

Conversation

BreiteSeite
Copy link
Contributor

@BreiteSeite BreiteSeite commented Jul 24, 2018

Fixes #63
Provide a narrative description of what you are trying to accomplish:

  • Are you fixing a bug?
    • Detail how the bug is invoked currently.
    • Detail the original, incorrect behavior.
    • Detail the new, expected behavior.
    • Base your feature on the master branch, and submit against that branch.
    • Add a regression test that demonstrates the bug, and proves the fix.
    • Add a CHANGELOG.md entry for the fix.


if (empty($rawBody)) {
return $request
->withAttribute('rawBody', $rawBody)
Copy link
Member

Choose a reason for hiding this comment

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

Why are you adding this attribute? The raw body can already be accessed via $request->getBody(), making this redundant information.

Copy link
Contributor Author

@BreiteSeite BreiteSeite Jul 24, 2018

Choose a reason for hiding this comment

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

@weierophinney Without my change, this was added as well, so i was keeping it to not introduce some kind of BC break. See: https://github.com/zendframework/zend-expressive-helpers/pull/64/files#diff-2def9921da214e2d356d90bbcf2ef2d9R60

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm... okay.

We should likely revisit that in the next major version, as we shouldn't be doing that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, good point. I opened #65 for that.

@weierophinney weierophinney merged commit 2d0edc4 into zendframework:master Jul 25, 2018
weierophinney added a commit that referenced this pull request Jul 25, 2018
weierophinney added a commit that referenced this pull request Jul 25, 2018
weierophinney added a commit that referenced this pull request Jul 25, 2018
Forward port #64

Conflicts:
	CHANGELOG.md
@weierophinney
Copy link
Member

Thanks, @BreiteSeite!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent unnecessary json_decode() call with empty request body
2 participants