Skip to content

Conversation

@SharkyKZ
Copy link
Contributor

Pull Request for Issue #27919.

Summary of Changes

Joomla\CMS\MVC\Controller\FormController::reload() method filters form data before storing it to user state. This is different from behavior in save() method where we store unfiltered data. Filtering data at this point causes some issues, e.g. form values disappearing if unset filter is used.

This removes filtering.

Testing Instructions

Create two or more categories.
Create a custom field for articles.
Edit an article.
Change its category.
After page is reloaded, check Hits and Revision fields.

Expected result

Values are present.

Actual result

Values are missing.

Documentation Changes Required

IDK.

@ReLater
Copy link
Contributor

ReLater commented Feb 27, 2020

What's about field Modified By? OK, that it's empty after category change and reload?

Isn't it a security issue if unfiltered data, e.g. JS-<script> in MetaDescription field is saved in the UserData/Session? If I don't save the article after category change and it's reloaded with <script> tag intact, keep article open, isn't it possible to access the data somewhere else? I don't know... Just asking...

@SharkyKZ
Copy link
Contributor Author

All of this is in line with how we handle data saving. Even the Modified By field issue (this will need to be fixed separately).

@ReLater
Copy link
Contributor

ReLater commented Feb 27, 2020

I have tested this item ✅ successfully on 3dadc5b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28103.

@zero-24
Copy link
Contributor

zero-24 commented Feb 27, 2020

hmm I'm not sure whether this is a good idea. In the save method we use validate before saving it to the user state: https://github.com/joomla/joomla-cms/pull/28103/files#diff-86b33cf8a55249ba67537358db4a53e1L714

@SniperSister please take a look here and give your advise.

@SharkyKZ
Copy link
Contributor Author

SharkyKZ commented Feb 27, 2020

@zero-24 No, we store unfiltered data in user state:

// Save the data in the session.
$app->setUserState($context . '.data', $data);

@jwaisner
Copy link
Member

I have tested this item ✅ successfully on 3dadc5b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28103.

@jwaisner
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28103.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 28, 2020
@HLeithner HLeithner merged commit ab67635 into joomla:staging Feb 28, 2020
@HLeithner
Copy link
Member

Thanks

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 28, 2020
@HLeithner HLeithner added this to the Joomla! 3.9.16 milestone Feb 28, 2020
HLeithner added a commit that referenced this pull request Apr 7, 2020
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.

6 participants