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

CMS get/post Vars are too generic #2822

Open
n8-dev opened this issue Jan 18, 2023 · 0 comments
Open

CMS get/post Vars are too generic #2822

n8-dev opened this issue Jan 18, 2023 · 0 comments

Comments

@n8-dev
Copy link

n8-dev commented Jan 18, 2023

Preface

Not certain if this will be considered a core fix, or a documentation gotcha that will need to be added.

Context

I noticed this situation on a site using https://github.com/silverstripe/silverstripe-blog

In site level extension there was some logic around getUrlTags() to filter posts.

There is a very generic getVar('Tags') being consumed.

This then gets passed to an explode as a string, and used to filter some DataObjects.

I've already had to implement a check for CMSPreview=1 a la silverstripe/silverstripe-framework#10602
to help ensure things are running when they should.

Due to the references of this function and where its also called when the Add New Blog Post button is clicked it runs.

This causes a problem.

The first load upon click of the Add New Blog Post is the following url:

/admin/pages/edit/EditForm/42/field/ChildPages?ChildPages[GridState]={"GridFieldSiteTreeAddNewButton":{"currentPageID":42,"pageType":"SilverStripe\\Blog\\Model\\BlogPost"}}&Categories[GridState]={}&Tags[GridState]={"GridFieldOrderableRows":{"enabled":true}}&action_gridFieldAlterAction?StateID=gf_3ac21a87=Add new Blog Post&SecurityID=xxxx

this then 302's to the newly created Blog Page when it runs correctly.

The Problem.

The problem is the cms and GridfieldState logic is also using the getVar Tags

In this situation, Tags is an array, which breaks the explode etc.

I've fixed this in the site level code by ensuring the filtering etc only runs on the correct Controller (not the AdminRootController)

Question / Solution

But it begs the question, how many over greedy functions may possibly collide with the core CMS getVars()?

Is there a reason these are not prefixed (e.g. CMSTags=) or nested within an array from CMS consumption (e.g. ?CMSVars=)`

These details didn't use to be present in past versions of SS either.
I understand the route of no change, but then perhaps we should have some documentation around special/reserved details perhaps?

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

No branches or pull requests

2 participants