I would be pleased to test, but I do not get the timeout locally on MAMP php 5.4.4

@photodude, @PhilETaylor could you make a PR to this PR adding the test case so that we have a prove that this here is working, Thanks

I have tested this item ✅ successfully on dd82ff2

Great, this has worked on the test scenario where it was previously failing for me.

And to confirm, you can also use:

JRequest::get('request', JREQUEST_ALLOWHTML);

with this fix too. Where it previously failed when a field contained the link text above.

Same with:
$filter = JFilterInput::getInstance(null, null, 1, 1);
$filter->clean($row, 'HTML');

Is also working now.

Many thanks
Tony

@ggppdk I tested this on my environment trying to save an article in FLEXIcontent and I get the following errors:

Notice: Undefined variable: attributeValueToEnd in libraries/joomla/filter/input.php on line 1103 (repeats thousands of times)

followed by error:

Fatal error: Maximum execution time of 60 seconds exceeded in libraries/vendor/joomla/string/src/phputf8/mbstring/core.php on line 41

Using the same save as before: https://gist.github.com/lyquix-owner/c8c189a016c3d99c1008059920a87787

@lyquix-owner did you replace the whole file with the one @ggppdk modified?

https://github.com/ggppdk/joomla-cms/blob/dd82ff21aeb1ad70abb1d4ddee4e77e8861d8584/libraries/joomla/filter/input.php
Line 1103 is: // We have a closing quote, convert its byte position to a UTF-8 string length, using non-multibyte substr()

which is not a variable field.

@infograf768 to get the time out you need certain combinations of tags and multibyte characters like Russian, Greek, Arabic, etc. There is a content file in issue #15673
linked here as well content.txt which will reproduce the issue. @ggppdk please include this file as part of your testing instructions.

There is a subsection of that content file which has been turned into a unit test at #15810 ( @ggppdk this is the unit test you need to include as mentioned by @rdeutz in #15966 (comment) )
Additionally, I have a few base case additions to the unit tests with multibyte characters open at #15914 and one merged in the framework joomla-framework/filter#24 which should also be included to verify that any changes do not break existing filtering.

Notice: Undefined variable: attributeValueToEnd in libraries/joomla/filter/input.php on line 1103

Similiar issues with travis

1) JFilterInputTest::testCleanByCallingMember with data set "Kill script" ('', '<img src="javascript:alert();" />', '', 'From specific cases')
Undefined variable: attributeValueToEnd

/home/travis/build/joomla/joomla-cms/tests/unit/core/helper.php:52
/home/travis/build/joomla/joomla-cms/libraries/joomla/filter/input.php:1104
/home/travis/build/joomla/joomla-cms/libraries/joomla/filter/input.php:1198
/home/travis/build/joomla/joomla-cms/libraries/joomla/filter/input.php:824
/home/travis/build/joomla/joomla-cms/libraries/joomla/filter/input.php:809
/home/travis/build/joomla/joomla-cms/libraries/joomla/filter/input.php:790
/home/travis/build/joomla/joomla-cms/libraries/joomla/filter/input.php:772
/home/travis/build/joomla/joomla-cms/libraries/joomla/filter/input.php:439
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/filter/JFilterInputTest.php:772

https://travis-ci.org/joomla/joomla-cms/jobs/231181384

@ggppdk you made a mistake in line 1104

'$stringBeforeQuote = substr($attributeValueToEnd, 0, $matches[0][1]);"

$attributeValueToEnd isn't set

@ggppdk does this fix the escaping case for 1>5 as listed in the framework as an issue? see unit test PR at joomla-framework/filter#15

@rdeutz @lyquix-owner @photodude fixed wrong variable name

I dont think this fixes everything as the unit tests still fail to finish https://travis-ci.org/joomla/joomla-cms/jobs/231213928

@rdeutz could you make a PR to this PR adding the test case so that we have a prove that this here is working, Thanks

for what its worth, my test case 15673.php from https://github.com/PhilETaylor/joomla-cms/blob/6438379dba018e9a3ac72de0c90a17420535eb61/15673.php works with joomla-cms/stable at b3a83cb while it did not work with Joomal 3.7.0 stable, so something has changed in staging already to fix that test case.

@PhilETaylor I meant the unit tests, added them to my file locally and tests are failing

My point is that without my crappy unit test, the current unit tests we had before, that are currently in staging, are failing see: https://travis-ci.org/joomla/joomla-cms/jobs/231213928

..................................................F..F..FF... 2074 / 5777 ( 35%)
............................................................. 2135 / 5777 ( 36%)
.........................F..F..FFFF..F....................... 2196 / 5777 ( 38%)
............................................................. 2257 / 5777 ( 39%)
....F..F..FF................................................. 2318 / 5777 ( 40%)
.....................................F.FFF...FF.............. 2379 / 5777 ( 41%)
............................................................. 2440 / 5777 ( 42%)
............FFFFFF..FFFFFF..FF.F.FFFF

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

So this PR obviously has made things worse than in Joomla 3.7.0, because at least passed the CURRENT set of unit tests...

@ggppdk the fixed wrong variable name didn't fix anything. The unit tests now fail to work.

I have fixed it, i hope unit tests will now pass

Yes! It worked this time on my end!

I have tested this item ✅ successfully on 7679cc2

do we need new tests?

@infograf768 let some more people test this

i meant new unit tests

I can merge @photodude tests, so we have some more testing. The real issue is hard to test because you are in an endless loop

@infograf768 @rdeutz we need the unit test from #15810 I've been testing this patch and the related unit test on the framework. This seems to solve the issue, but the unit test needs work to be valid.

Here are the results from my tests of this patch with #15810 https://travis-ci.org/photodude/filter/jobs/231328081

I have tested this item ✅ successfully on 7679cc2

I've tested successfully with the below test case to reproduce the timeout.
Important is to login with a user member of the Manager group, create a new article cut&paste the below 4 lines:
Test with user group Manager-articles 19 février 2017 mélanie propose

after read more 19 février 2017 mélanie propose
Voilà
./jluc
Then go at the end of the first line add ReadMore (button), Save and/or Save&Close produce the timeout. The patch fix the issue.

In the above test case, Github skinked the needed spaces, code available at
https://gist.github.com/jsubri/6d9577cc8b183513d2be9bead005fece

RTC after two successful tests.

@ggppdk please can you merge these regression test cases ggppdk#4 then we can get this merged :)

merged it now we can add test cases later