Fix previous and next helpers #5180
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fixes TryGhost#5177 - we now pass API/model options directly to toJSON, which is unsafe as these options haven't always been filtered before they are passed. - this fix adds a filter so that toJSON only uses the options it needs - additionally, rename the 'name' option to something more specific to prevent clashes
fixes TryGhost#5177 - this combined with a change passing options through to toJSON results in a really flukey bug with next/prev where the name option from the helper clashes with a name option inside of toJSON
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #5177
This bug is caused by an interesting collection of oddnesses. All of the tests for the next/previous API calls and for the helper are passing, and that's not strictly incorrect. However the call the next/prev helper makes to the API includes all of the options from the helper, including one called
name.In #5159, I changed the API/models to always pass options through to
toJSON. However, those options are only filtered in the model layer, not the API layer, so in cases where the API callstoJSON(E.g. theposts.readcall used in the next/prev helpers) the options passed in are unsanitized. This was a massive oversight on my part.Deep inside of
toJSON, there is some code to handle nested relations. This code makes use of anameoption that gets passed recursively totoJSONin order to create nested keys.As a result of the combination of these things: options from the helper getting passed through to the API, not being filtered, and then being used in toJSON the
nameproperty from the helper was incorrectly being used as the base for the key intoJSON, meaning that the next and previous posts were being casually thrown away before the API returned. Epic.This PR contains two separate fixes for the two separate issues. The first one adds filtering to
toJSONso that only allowed options are passed through. It also renamesnametobaseKeywhich is more specific and less likely to clash with other things. The second one changes the prev/next helper to not pass through the options from the helper to the API.Normally I'd do these as 2 separate PRs, but in the interest of expediency (cough travis cough I've combined them into one).