preg_replace segmentation fault causes a system crash when viewing a node [#2510774]

When there are many words in the text matching the word to replace, i get a "connection reset" error in firefox ("no data received" in chrome) after debugging I found it's a segmentation fault error, and it's caused for the great number of recursions, I checked the code and the problem is in the pattern applied, I made a patch to change it a little bit and it works, the original pattern the module uses is:
'$\b(WordToFind)\b(?!((?!alink_replaced).)*alink_replaced</a>)$ui'
and the modified one is:
'$\b(WordToFind)\b(?!((?!alink_replaced).*)alink_replaced</a>)$ui'
It's a very slightly change but it gets the job done, please check this and review it to find if there are any associated problems.

Comment	File	Size	Author
#6	alinks-preg_replace_system_crash-2510774-2.patch	874 bytes	santiwww
#2	alinks-preg_replace_system_crash-2510774-1.patch	900 bytes	santiwww

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Greg Boggs

he/him

English

Portland Oregon

CreditAttribution: Greg Boggs as a volunteer commented 23 June 2015 at 16:23

Thanks for the improvement. My regex is a bit rusty. Do you know what the change does? Are there any edge cases I should test for?

Comment #2

santiwww CreditAttribution: santiwww commented 23 June 2015 at 16:24

File	Size
alinks-preg_replace_system_crash-2510774-1.patch	900 bytes

Comment #3

santiwww CreditAttribution: santiwww commented 23 June 2015 at 16:30

To be honest i'm not sure what it does... :-D
I found this related questions after long testing and debugging:
http://stackoverflow.com/questions/4157743/preg-replace-throws-seg-fault
http://stackoverflow.com/questions/20750757/php-segmentation-fault-durin...
I think it has to be related to recursion limit of PCRE, but I'm not sure why sometimes it fails.
I tested with a text from a node with more than 10 words, but I'm not sure if different system and php configurations have the same problem or not.

Comment #4

Greg Boggs

he/him

English

Portland Oregon

CreditAttribution: Greg Boggs as a volunteer commented 23 June 2015 at 16:44

Ok. It looks pretty harmless, but regex can be tricky. My plan after I test this is to roll it into the dev release to see if I can get a few testers. I'll update this ticket should any problems be found.

Comment #5

santiwww CreditAttribution: santiwww commented 20 August 2015 at 08:10

Problem found, I increased the number of alinks in my installation, and also i use different weights for some of them, after that I checked if everything was working fine and found that in a page with multiple words to be replaced, only one of them was working, so i decided to roll back the modification i made and it started to work again, but the segmentation fault error appears again.
Now I'm testing with this expression:
'$\b(WordToFind)\b(?!alink_replaced))$ui'
And it works but I'm far to be an expert in regex, and I don't fully understand why it was done that way, so test is needed in order to be sure there are no side effects.