Skip to content

Stop the page Content tab from shadowing the content field (#4271) - #4289

Open
wakqasahmed wants to merge 1 commit into
getgrav:developfrom
wakqasahmed:fix/issue-4271-blueprint-content-key-collision
Open

Stop the page Content tab from shadowing the content field (#4271)#4289
wakqasahmed wants to merge 1 commit into
getgrav:developfrom
wakqasahmed:fix/issue-4271-blueprint-content-key-collision

Conversation

@wakqasahmed

Copy link
Copy Markdown
Contributor

Fixes #4271

In system/blueprints/pages/default.yaml, the Content tab and the markdown content field are both keyed content. BlueprintSchema flattens every field into one map keyed by name, containers included, and it does this by writing the field's own entry after recursing into its children, so the tab's entry gets written last and wins. The markdown field's validate config sits in that map under content too, but it's never the one that gets read back, so it never actually runs.

I renamed the tab's key to content_tab and left the field's key alone, since content is the actual page body key used throughout Grav and themes and isn't safe to touch. modular.yaml and external.yaml both extend default.yaml and override that same tab by key, so I updated them to content_tab too. Otherwise their overrides would stop merging into the tab and would themselves collide with the content field the same way.

I checked how much this actually changes at runtime before assuming a rename alone was safe. checkSafety() (the XSS scan) already runs on this field today regardless of which rule wins, since it only skips fields typed unset, so no new XSS behavior gets switched on here. Type validation is the part that starts working, and the field already has max: 0 set from #3643 to opt out of the length cap, so in practice this now only requires content to be a string, which is what a textarea always posts anyway.

Added a test that loads the real pages/default.yaml blueprint and asserts the flattened content item resolves to the markdown field's type and validate config rather than the tab's.

Ran the existing blueprint/data tests plus the full page test suite locally, all green, and confirmed the new test fails against the old blueprint (resolves to type tab) and passes with the fix.

getgrav#4271)

BlueprintSchema flattens every field into a single map keyed by name,
containers included, so the tabs.fields.content tab and the nested
markdown content field in pages/default.yaml collided on the same key
and the tab (assigned after its own fields are parsed) always won.
That left the content field's validate config unreachable, so page
body content was never actually validated.

Rename the tab's key to content_tab and update modular.yaml and
external.yaml, which extend default.yaml and override that same tab,
to match. The content field's key is untouched since it is the real
page body key referenced throughout Grav and themes.

Add a regression test that loads the real pages/default.yaml blueprint
and asserts the flattened content item is the markdown field, not the
tab, and that its validate config is reachable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Page blueprint's content field is shadowed by the content tab, so page content is never validated

1 participant