Skip to content

Related Posts: Fix fatal when the exclusion parameter is not a string#6922

Merged
zinigor merged 1 commit intoAutomattic:masterfrom
xyu:fix/related-posts-excluded-ids
Apr 24, 2017
Merged

Related Posts: Fix fatal when the exclusion parameter is not a string#6922
zinigor merged 1 commit intoAutomattic:masterfrom
xyu:fix/related-posts-excluded-ids

Conversation

@xyu
Copy link
Copy Markdown
Member

@xyu xyu commented Apr 5, 2017

Some requests for related posts are using a malformed URL where the
relatedposts_exclude parameter is of type array, not of type string, causing
explode() to fatal.

This change checks the type of the relatedpost_exclude parameter, and will
perform the original explode() if it is a string; or extract the values if
it's an array. We sanitize the values to ensure they are valid ints for use as
$excludes.

Props @jmdodd

Fixes #N/A

Changes proposed in this Pull Request:

  • Sanitize values and handle both comma delimited values as well as url arrays

Testing instructions:

  • Check that the URL param relatedposts_exclude[]=X and relatedposts_exclude=X both exclude post_id X.

Some requests for related posts are using a malformed URL where the
`relatedposts_exclude` parameter is of type array, not of type string, causing
`explode()` to fatal.

This change checks the type of the `relatedpost_exclude` parameter, and will
perform the original `explode()` if it is a string; or extract the values if
it's an array. We sanitize the values to ensure they are valid ints for use as
`$excludes`.

Props @jmdodd
@xyu
Copy link
Copy Markdown
Member Author

xyu commented Apr 5, 2017

Changes have already been deployed to WPCOM side.

@dereksmart dereksmart added the Bug When a feature is broken and / or not performing as intended label Apr 5, 2017
@dereksmart dereksmart added the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 5, 2017
@dereksmart dereksmart added this to the 4.8.1 milestone Apr 5, 2017
@dereksmart
Copy link
Copy Markdown
Contributor

Travis is failing because it can't use yarn for some reason (again), but this PR is fine to merge since it only introduces PHP changes and all PHP tests still pass.

@jeherve jeherve modified the milestone: 4.8.1 Apr 6, 2017
@eliorivero eliorivero added this to the 4.8.2 milestone Apr 6, 2017
@jeherve jeherve removed this from the 4.8.2 milestone Apr 7, 2017
@jeherve jeherve modified the milestone: 4.9 Apr 24, 2017
@zinigor zinigor merged commit ed04b59 into Automattic:master Apr 24, 2017
@zinigor zinigor removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 24, 2017
jeherve added a commit that referenced this pull request Apr 24, 2017
eliorivero pushed a commit that referenced this pull request Apr 25, 2017
* Changelog: initial commit for 4.9 release.

* Changelog: add #6929

* Changelog: move old changelogs to changelog.txt

* Readme: restore deleted release post link.

The post is now live.

* Changelog: add #6853

* Changelog: add #6856

* Changelog: add #6857

* Changelog: add #6884

* Changelog: add #6885

* Changelog: add #6892

* Changelog: add #6894

* Changelog: add #6898

* Changelog: add #6899

* Changelog: add #6900

* Changelog: add #6909

* Changelog: add #6927

* Changelog: add #6947

* Chagelog: add #6958

* Changelog: add #6961

* Changelog: add #6963

* Changelog: add #6965

* Changelog: add #6986

* Changelog: add #7000

* Changelog: add #7013

* Changelog: add #7015

* Changelog: add #7019

* Changelog: add #7028

* Changelog: add #6998

* Changelog: add #6999

* Changelog: add #7044

* Changelog: add #6881

* Changelog: add #6922

* Changelog: add #6940

* Changelog: add #6962

* Changelog: add #6942

* Changelog: add #6959

* Changelog: add #7018

* Changelog: add #6948

* Changelog: add #6657

* Changelog: add #7030

* Changelog: add #7048

* Changelog: add #7031

* Changelog: add #6990

* Changelog: add #6957

* Changelog: add #7027
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 [Feature] Related Posts [Pri] High Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants