[#31128] - [4.0]Make sure we do not add one header twice but we support to set a different header per client.
- Fixed in Code Base
- 14 Nov 2020
- Medium
- Build: 4.0-dev
- # 31128
- Diff
- zero-24:30962
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Success Download Prebuilt packages are available for download. Details
- Success continuous-integration/drone/pr Build is passing Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #30962 cc @PhilETaylor
Summary of Changes
Make sure we do not add one header twice but we support to set a different header per client.
Testing Instructions
Setup Plugins -> System - HTTP Headers as this screenshot:
Actual result BEFORE applying this Pull Request
Content-Security-Policy: TWO
Expected result AFTER applying this Pull Request
Content-Security-Policy: ONE
Documentation Changes Required
None
@PhilETaylor I'm not sure whether we should block the save as it could be valid to configure two headers with the same value. e.g. one for the backend and one for the frontend. With this change here we make sure only the first configured per site (or for both) is choosen. I'm not sure how to cleanly validate the subform at that point other than hooking into the onExtensionBeforeSave Event but than this pugin would be triggered on all extension saves does not sound that optimal to me?
Votes
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End Plugins |
hi, the PR works but why not having a javascript check to warn the user that the same header with the same client is already set when fill the second one? There might more even more effective solutions (like to disable options when already set) but probably not worth the effort.
I think it's useful to at least inform the user that what he has set (multiple values for same header/client) is not what he will get (correctly with this patch but we need to inform the user of the "mistake" if not prevent)
the PR works but why not having a javascript check to warn the user that the same header with the same client is already set when fill the second one?
hmm I get the point but I'm no JS expert do you know how to implement such check in the subform? The other way around could be a validation rule I guess.
I have tested this item
It is not so obvious that you look for the result in the headers sent. I was expecting the form to changes! Anyway, I used my browser HTTP Header Live tool and could see the the change from TWO to ONE as described.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31128.
hmm I get the point but I'm no JS expert do you know how to implement such check in the subform? The other way around could be a validation rule I guess.
yes, maybe better validate server-side the configuration of the plugin when saving and not accepting the save operation when parameters, also displaying an error message
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31128.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31128.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31128.
There is a strange error shown in drone for javascript-cs, but that's not related to this PR.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-11-14 13:15:31 |
| Closed_By | ⇒ | richard67 | |
| Labels |
Added:
?
|
||
Thanks!
I have tested this item✅ successfully on a004fa3
done :)
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31128.