Skip to content

REST API: Add support for HTTP_CONTENT_TYPE in authentication#6095

Merged
dereksmart merged 1 commit intomasterfrom
fix/jetpack-rest-authenticate-content-type
Jan 13, 2017
Merged

REST API: Add support for HTTP_CONTENT_TYPE in authentication#6095
dereksmart merged 1 commit intomasterfrom
fix/jetpack-rest-authenticate-content-type

Conversation

@tyxla
Copy link
Copy Markdown
Member

@tyxla tyxla commented Jan 13, 2017

In #5418 we introduced an alternative authentication method that allows us to authenticate with Jetpack Signatures.

However while testing together with @oskosk we discovered that there's a bug with the Content-Type. Specifically, we're reading it only from $_SERVER['CONTENT_TYPE'], but some servers might pass it as $_SERVER['HTTP_CONTENT_TYPE']. This PR adds support for HTTP_CONTENT_TYPE in addition to CONTENT_TYPE. In addition, we're making sure that the content type is actually specified (by using ! empty() instead of isset()).

Thanks @oskosk for finding this one.

Testing instructions

Please review the code. The fix here appears to be a common practice with PHP development, as we always need to consider BOTH keys under $_SERVER for correct content type detection.

Proposed changelog entry for your changes:

Fixes detection of request content type, based on available headers passed down by the web server.

@tyxla tyxla added [Feature] WPCOM API [Pri] High Bug When a feature is broken and / or not performing as intended labels Jan 13, 2017
@tyxla tyxla self-assigned this Jan 13, 2017
@tyxla tyxla added the [Status] Needs Review This PR is ready for review. label Jan 13, 2017
@oskosk
Copy link
Copy Markdown
Contributor

oskosk commented Jan 13, 2017

Tested, both with a Jetpack site installation running over HTTP and another installation running over HTTPS.

LGTM

@oskosk oskosk added [Status] Ready to Merge Go ahead, you can push that green button! REST API and removed [Feature] WPCOM API [Status] Needs Review This PR is ready for review. labels Jan 13, 2017
@oskosk oskosk added this to the 4.5 milestone Jan 13, 2017
@oskosk
Copy link
Copy Markdown
Contributor

oskosk commented Jan 13, 2017

@dereksmart when you have some time, please confirm that the milestone 4.5 for this PR is correct. Thanks!

@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Jan 13, 2017

Yep, I agree, LGTM after confirming that this goes into 4.5.

@dereksmart dereksmart merged commit 997a48a into master Jan 13, 2017
@dereksmart
Copy link
Copy Markdown
Contributor

Into 4.5 it goes! af7d4b7

@dereksmart dereksmart deleted the fix/jetpack-rest-authenticate-content-type branch January 13, 2017 14:08
@tyxla
Copy link
Copy Markdown
Member Author

tyxla commented Jan 13, 2017

That was fast! Thanks 🙇

dereksmart pushed a commit that referenced this pull request Jan 17, 2017
CHangelog: add #5457

Changelog: add #5487

Changelog: add #5708

Changelog: add #5879

Changelog: add #5932

Changelog: add #5963

Changelog: add #5968

Changelog: add #5996

Changelog: add #5998

Changelog: add #5999

Changelog: add #6012

Changelog: add #6013

Changelog: add #6014

Changelog: add #6015

Changelog: add #6023

Changelog: add #6024

Changelog: add #6030

Changelog: add #5465

CHangelog: add #6063

Changelog: add #6025

Changelog: add #5974

Changelog: add #6059

Changelog: add #6046

Changelog: add #5418

Changelog: move things around and add missing information.

Changelog: add #5565

Changelog: add #6087

Changelog: add #6095
dereksmart pushed a commit that referenced this pull request Jan 17, 2017
Changelog: add #5867

Changelog: add #5874

Changelog: add #5905

Changelog: add #5906

Changelog: add #5931

Changelog: add #5933

Changelog: add #5934

Bring over 4.4.2 changelog from branch-4.4

@see 18012a3

Changelog: add #5976, #5978, #5983

Changelog: add #5917

Changelog: add #5832

Changelog: add 4.4.2 release post link.

CHangelog: add #5457

Changelog: add #5487

Changelog: add #5708

Changelog: add #5879

Changelog: add #5932

Changelog: add #5963

Changelog: add #5968

Changelog: add #5996

Changelog: add #5998

Changelog: add #5999

Changelog: add #6012

Changelog: add #6013

Changelog: add #6014

Changelog: add #6015

Changelog: add #6023

Changelog: add #6024

Changelog: add #6030

Changelog: add #5465

CHangelog: add #6063

Changelog: add #6025

Changelog: add #5974

Changelog: add #6059

Changelog: add #6046

Changelog: add #5418

Changelog: move things around and add missing information.

Changelog: add #5565

Changelog: add #6087

Changelog: add #6095

Readme: add @tyxla to the list of contributors.

Improved changelog for your readability and enjoyment

updated the release date

finalizing the changelog with a few more edits
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended [Pri] High

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants