Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added support to parse json request to Request::getRestParams() #2065

Merged
merged 1 commit into from Jan 19, 2014

Conversation

@cebe
Copy link
Member

@cebe cebe commented Jan 19, 2014

cebe referenced this pull request Jan 19, 2014
…web\Request::acceptedContentTypes` to `acceptableContentTypes`.

Renamed `yii\web\Request::acceptedLanguages` to `acceptableLanguages`
qiangxue added a commit that referenced this pull request Jan 19, 2014
added support to parse json request to Request::getRestParams()
@qiangxue qiangxue merged commit 12eca51 into yiisoft:master Jan 19, 2014
1 check passed
1 check passed
default The Travis CI build passed
Details
@qiangxue
Copy link
Member

@qiangxue qiangxue commented Jan 19, 2014

Looks great to me!

@@ -256,6 +257,8 @@ public function getRestParams()
if ($this->_restParams === null) {
if (isset($_POST[$this->restVar])) {
$this->_restParams = $_POST;
} elseif(strncmp($this->getContentType(), 'application/json', 16) === 0) {
$this->_restParams = Json::decode($this->getRawBody(), true);

This comment has been minimized.

@klimov-paul

klimov-paul Jan 19, 2014
Member

What if would be "text/json"?

This comment has been minimized.

@klimov-paul

klimov-paul Jan 19, 2014
Member

Of course, application/json is canonical and perhaps the only one correct.
However, you will wonder how many applications which use different mime type for it exist.
I assume we can just check if mime type is ending on /json instead of full comparison.

This comment has been minimized.

@nineinchnick

nineinchnick Jan 19, 2014
Contributor

Yii 1 got a nice method in 1.1.14 to parse mime types. You could just compare the sub type and ignore type (the first part).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.