[#16875] - New way for input getString and getHtml
- Closed
- 4 Nov 2017
- Medium
- Build: staging
- # 16875
- Diff
- csthomas:fixfilter4
- Success hound No violations found. Woof! Details
- Success continuous-integration/drone/pr the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
User tests: Successful: Unsuccessful:
Pull Request for Issue ##16812
I'm not sure if this is correct. This is my initial concept.
Summary of Changes
- New way for
getString()method:
getString()- always remove all tags
Above idea is borrowed from #16842
- New way for
getHtml()method:
getHtml()- escapes<and>instead remove it:
- if character
<is followed by a white space, ex4 < 5then it will be replaced by4 < 5 - if
>does not have paired<, ex:A>B=>A>B - if
<>then that characters will be replace by<>??? - if
<does not have a pair with>then it will be replaced by<, ex:A<B=>A<B
- Fix notice:
Notice: A non well formed numeric value encountered in .../libraries/joomla/filter/input.php on line 1202 divHello "Joomla"
tested as:
$_REQUEST['test'] = '<div>Hello "Joomla"</div>';
echo JFactory::getApplication()->input->getString('test', '');die;
Testing Instructions
Not yet.
I suspect that above point 2 could not be accepted in full. Any comments?
Expected result
getString() will always return clean text without any tags. Input whitelist and blacklist will not change that.
Actual result
...
Documentation Changes Required
Probably.
Details
- For filter instance with default blacklist enabled:
| No. | Input | Type | Old behaviour* | This PR |
|---|---|---|---|---|
| 1. | '<em' |
'STRING' |
'em' |
'' |
'<em' |
'' |
'em' |
'<em' |
|
'<em' |
'HTML' |
'em' |
'<em' |
|
'<em' |
'STRING' |
'em' |
'' |
|
'<em' |
'' |
'em' |
'<em' |
|
'<em' |
'HTML' |
'<em' |
'<em' |
|
| 2. | 'em>' |
'STRING' |
'em>' |
'em>' |
'em>' |
'' |
'em>' |
'em>' |
|
'em>' |
'HTML' |
'em>' |
'em>' |
|
'em>' |
'STRING' |
'em>' |
'em>' |
|
'em>' |
'' |
'em>' |
'em>' |
|
'em>' |
'HTML' |
'em>' |
'em>' |
|
| 3. | '< ' |
'STRING' |
' ' |
'< ' |
'< ' |
'' |
' ' |
'< ' |
|
'< ' |
'HTML' |
' ' |
'< ' |
|
'< ' |
'STRING' |
' ' |
'< ' |
|
'< ' |
'' |
' ' |
'< ' |
|
'< ' |
'HTML' |
'< ' |
'< ' |
|
| 4.** | '<>' |
'STRING' |
'' |
'' |
'<>' |
'' |
'' |
'<>' |
|
'<>' |
'HTML' |
'' |
'<>' |
|
'<>' |
'STRING' |
'' |
'' |
|
'<>' |
'' |
'' |
'<>' |
|
'<>' |
'HTML' |
'<>' |
'<>' |
|
(*) Old behaviour means behaviour on J3.7.2 and older.
(**) I'm going to revert behaviour for point 4 on this PR.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries Unit Tests |
I am not going to merge this. After some discussion I came to the conclusion that we need to look deeper into this and that we don't have the time to do this before the next release will be out.
@rdeutz If this issue #16842 is going to be introduced in the next 3.7.3 update it should be at least documented as potential issues can arise. For example our extensions need an update because the '>' character is stripped out by getString.
If we don't have time to get it definitely fixed for 3.7.3, at least it would be the case to revert the getString method to still allow the '>' character as in 3.7.2 to avoid bugs.
I wrote small PR to fix only B/C break in input filter. It is not complicated as this one.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-11-04 12:25:43 |
| Closed_By | ⇒ | csthomas |
Sounds good to me.
Just tested the getString method and it works as expected. Now it's possible to preserve '<' and '>' characters if used in a generic context.