Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot Save or Update Long Articles #16112

Closed
lynnfredricks opened this issue May 18, 2017 · 38 comments
Closed

Cannot Save or Update Long Articles #16112

lynnfredricks opened this issue May 18, 2017 · 38 comments

Comments

@lynnfredricks
Copy link

lynnfredricks commented May 18, 2017

Steps to reproduce the issue

  1. Create or duplicate a large article such as this:
    http://www.silkypix.us/silkypix-supported-cameras

  2. Try to save the article.

Expected result

Saved article, as had worked under 3.6.5.

Actual result

White page with:
Fatal error: Maximum execution time of 30 seconds exceeded in /home/XXXX/XXXX/libraries/vendor/joomla/string/src/phputf8/mbstring/core.php on line 41

System information (as much as possible)

PHP 5.6.29
Joomla 3.7.1

Additional comments

The symptoms of this were present in 3.7.0 and merged into another item. The other item was listed as fixed and CLOSED, but updating to 3.7.1 did not solve the problem.

@PhilETaylor

This comment was marked as abuse.

@lynnfredricks
Copy link
Author

Raw html of content article now attached at text. This is the article as it currently is. Any modification and attempt to save produces the problem.

@PhilETaylor

This comment was marked as abuse.

@brianteeman
Copy link
Contributor

I just tested on multiple systems using php 7.1.1 and php 5.6.30 and it saved with no problem at all

@lynnfredricks
Copy link
Author

PHP portion is subject to the whims of our provider - short term answer is no, cannot upgrade at present. However it is possible to upgrade in the future.

@PhilETaylor

This comment was marked as abuse.

@lynnfredricks
Copy link
Author

lynnfredricks commented May 18, 2017

So this means you are increasing the minimum requirements of Joomla 3.7.1 to php 5.6.30?

@brianteeman
Copy link
Contributor

No that means that Phil was expressing his personal opinion

@PhilETaylor

This comment was marked as abuse.

@brianteeman
Copy link
Contributor

I also has no issue saving your text on 5.5.38

@lynnfredricks
Copy link
Author

@brianteeman I accept @PhilETaylor opinion that backwards compatibility can suck.

@brianteeman
Copy link
Contributor

my point is that your problem is probably not purely php version related

@mbabker
Copy link
Contributor

mbabker commented May 18, 2017

If I had my way Joomla 4 would be PHP 7.1 only as a minimum.

FWIW we left the door open to revisit this all the way up to 4.0 beta 1 being released, so there's a lot of time to revisit and rethink that. Personally, I don't see us keeping PHP 5.5 support given the way our numbers are moving, but dropping everything before 7.1 is borderline pushing our luck right now.

@brianteeman
Copy link
Contributor

its all interesting but off topic

@ggppdk
Copy link
Contributor

ggppdk commented May 18, 2017

my point is that your problem is probably not purely php version related

Yes, it is the size of the article that has this effect.

in my not so old PC, it takes about 80 seconds to save the article
(plus a few seconds to reload the form)

Out of which the JFilterInput::cleanTags on the article text were about 75 seconds

  • about 5 seconds were JFilterInput::escapeAttributes() called at the beginging of JFilter::cleanTags()
  • and about 70 seconds were the main loop of JFilterInput::cleanTags() ...

Filtering code does not seem broken in this case,

  • just new filtering code has gotten much slower than J3.6.5

maybe 2x or 3x slower or more ? (i do not know , i have not tested with J3.6.5 (maybe tomorrow),
maybe someone else can test performance difference with J3.6.5

Article text is about 300KBytes with a lot of tags (and tag attributes?), and server is probably a bit slow
test with a timeout of 90 seconds to 150 seconds,
and it should save ...

@PepeLopez
Copy link

Having same issues, with something special:
Using a pagebreak within an article, containing special characters (in my case, germans like ÄÜÖ or ß ) the string validation runs into death:

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

Feedback of some editors:
Chrome / FF on Win10 are failing when trying to save the article
Cent browser (chromium based) is working well
Chrome / FF on Win7 are doing well, too.

It only appears when having special characters in the pagebreak content. Example:
<hr title="Äußerungen" class="system-pagebreak" alt="Äußerungen" />

When changing the special characters like this:
<hr title="Aeusserungen" class="system-pagebreak" alt="Aeusserungen" />
everthing's fine with saving.

Special characters in other parts of the article are no problem so far!

What's wrong there?

@PhilETaylor

This comment was marked as abuse.

@PepeLopez
Copy link

@PhilETaylor
Yes, Joomla 3.7.1 is installed!

However, I found out:
The error only appears, when the editor is NOT "administrator". When the user is NOT admin, the problem occurs. If admin, no error appears.

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@PepeLopez
Copy link

IIRC different filters are applied depending on user group/acl

Maybe, but it until update (from 3.6.5) there was no such problem!? However, what should I look for then? I mean, that specific call causes an 180s (and maybe "forever") endless loop for looking up the pagebreak thing.
Wrong group/acl or not: It shouldn't die this way!

@PhilETaylor

This comment was marked as abuse.

@mbabker
Copy link
Contributor

mbabker commented May 19, 2017

until update (from 3.6.5) there was no such problem

Right. Because in 3.7.0 an extra layer of code was added to our filtering system to better handle multibyte characters. Clearly this has come with performance issues, which we are trying to rectify. But we cannot simply just revert that change as it also had security implications.

@PepeLopez
Copy link

@mbabker
I'm fine with improvements. I just see, that it needs around 8 seconds to save an article with administrator rights, and not-working as "Manager" or below.
The only fix for me in this case is obviously to grant all editors admin rights - which is a bad idea as you may know :D
So it's pretty strange about this effect... keep in mind: Only the pagebreak seems to be affected, and only non-admins.
@PhilETaylor consider that for your tests, please. It's ok if it needs more time, but my case is a dead end, it's not about performance here (joomla is running on 16cores with 192g ram).

@lynnfredricks
Copy link
Author

@PhilETaylor I have added it back. Not sure what happened but I didn't remove it to begin with.

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@mbabker
Copy link
Contributor

mbabker commented May 19, 2017 via email

@csthomas
Copy link
Contributor

csthomas commented May 19, 2017

I think I found a solution. Where to write a unit test code? Which method?

@csthomas
Copy link
Contributor

Never mind, I created PR #16140

@ggppdk
Copy link
Contributor

ggppdk commented May 19, 2017

@PhilETaylor , thanks for ping me


It looks like some of the work done by @ggppdk https://github.com/ggppdk in the escapeAttributeValues method is at play here. Maybe he would like to comment on this and test it.

Wrong ! , my PR, besides fixing the meaningless addition
and besides renaming a variable and changing a comment

  • changed none of the logic of the methods !
  • and even if a fix for BUG 1 reveals a BUG 2 that does not mean the fix was invalid

PR #16140 made by @csthomas, proves my argument, the faulty code was not added by my PR, it prexisted, and PR #16140 does not revert any of my changes


I cannot create a unit test to replicate the issue at the moment, as the wrong filters run during unit testing, and not the filters a PUBLISHER has when they save an article.

Wrong ! (see PR #16140 again) escapeAttributeValues() runs regardless of which TEXT filters you have, you can add a test case to the unit tests

  • because escapeAttributeValues() is running before the loop to remove and check the Tags

and cleanTags that calls it , will run regardless if filtering for administrator or for publisher


It then goes into a loop.. until the php.ini max_execution_time is reached. This is not a performance issue, PHP version issue, it is replicate-able on a PHP 7.1/Joomla 3.7.1/Mac - Its a endless loop issue

I hope you are not confusing the performance issue with the parsing issue
just it sounded like this

@infograf768
Copy link
Member

infograf768 commented May 20, 2017

I do confirm a huge difference when saving as a Manager or as a SuperAdmin even after merging #16140
See #16140 (comment)

php.ini settings
max_execution_time = 300 ; Maximum execution time of each script, in seconds
max_input_time = 60 ; Maximum amount of time each script may spend parsing request data
memory_limit = 132M ; Maximum amount of memory a script may consume (8MB)
max_input_vars = 2786

@lynnfredricks
Copy link
Author

Any further movement on this? Just in case, I saw that in another report that Jsitemap Pro Pingomatic was causing a problem. On this specific site, the only third party extensions are the two main Akeeba ones.

@ghost
Copy link

ghost commented May 23, 2017

@lynnfredricks please have a Look and Test on #16201

@lynnfredricks
Copy link
Author

lynnfredricks commented May 23, 2017

@franz-wohlkoenig I just tested with 3.7.2 and execution time is not improved enough. Error has changed through:
/libraries/vendor/joomla/string/src/phputf8/mbstring/core.php on line 94

Same article, just tried saving it.

@ghost
Copy link

ghost commented May 23, 2017

@lynnfredricks can you please post comment on PR on PR #16201 itself?

@joomla-cms-bot
Copy link

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/16112

@ghost
Copy link

ghost commented May 24, 2017

closed ashaving PR #16201


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants