View Issue Details

IDProjectCategoryView StatusLast Update
0014447mantisbtotherpublic2014-09-23 18:05
ReporterHenrik Morgenbrodt Assigned Todregad  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.2.11 
Target Version1.2.12Fixed in Version1.2.12 
Summary0014447: URLs longer than 152 characters are causing problems
Description

Users did report sometimes a description was not shown.
When they edit the bug the text was back again.

Copying an exampletext we were able to reproduce this for other fields (additonal information, bugnotes) as well.

Seems to be caused by long URLs with more than 152 characters in total.
Details in "Steps to reproduce"

I expect my next note to be empty; it will include an URL with 153 characters

Steps To Reproduce

To reproduce you might want to use one of the below URLs and ADD ONE random character.

http://esolut.de?einparametermitlaaangemwertumdieURLlangzumachen=1111111111111111111111111111111111111111111111111111111111111111111111111111111111111x
http://esolut.de/somelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathso

TagsNo tags attached.
Attached Files
long_url_warning.png (14,054 bytes)   
long_url_warning.png (14,054 bytes)   

Relationships

related to 0012781 closeddregad Links in the comments look broken 
related to 0015721 closedgrangeway Functionality to consider porting to master-2.0.x 
has duplicate 0014654 closeddregad Bugnotes disappear when contains a very long link 
has duplicate 0015202 closeddregad string_insert_hrefs() returns NULL in case of long URLs in text 
has duplicate 0015462 closedatrol At times, content in text fields in a ticket is hidden completely when ticket is read-only 
related to 0014547 closeddregad Email hrefs in bugnotes should include the 'mailto:' prefix 

Activities

Henrik Morgenbrodt

Henrik Morgenbrodt

2012-07-03 09:20

reporter   ~0032231

Last edited: 2012-07-06 16:53

http://esolut.de/somelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsom

Henrik Morgenbrodt

Henrik Morgenbrodt

2012-07-03 09:21

reporter   ~0032232

I was right. the bugnote before seems to be empty.
In fact it contains an URL thats longer than 152 characters.
It shows again when i edit the bugnote.

atrol

atrol

2012-07-03 10:28

developer   ~0032234

This is caused by function string_insert_hrefs which is called if "URL Processing" is enabled (the default) in standard plugin MantisCoreFormatting

dregad

dregad

2012-07-03 10:36

developer   ~0032235

Last edited: 2012-07-03 10:49

thanks atrol, I was trying to figure out why it reproduced here, but not on my dev box :)

EDIT: actually, this is how my local box is setup and the long URLs display properly so there must be something else

dregad

dregad

2012-07-03 12:06

developer   ~0032236

The only explanation I can think of at the moment is an issue with PHP. I'm on 5.3.10 and it's working fine. What version are you on ?

The problem seems to be with this code, line 497 in string_api.php <pre>
$p_string = preg_replace_callback(
$s_email_regex,
create_function(
'$p_match',
'if( 0 === preg_match( "/' . $t_url_protocol . '/", $p_match[0] ) ) {
return \'\' . $p_match[0] . \'\';
} else {
return $p_match[0];
}'
),
$p_string );
</pre>

dregad

dregad

2012-07-03 12:23

developer   ~0032237

Last edited: 2012-07-03 12:28

Reproducing issue outside of mantisbt on my box: <pre>
$regex="/(([[:alpha:]][-+.[:alnum:]]):\/\/.?)?([a-z0-9!#+\/=?^_{|}~-]+"
. "(?:.[a-z0-9!#
+\/=?^_{|}~-]+))"
. "\@((?:[a-z0-9](?:[a-z0-9-]
[a-z0-9])?.)+a-z0-9?)/i";
$url='http://example.com/';
for($i=0;$i<1000;$i++) $url.='a';
$str=''.$url.' ['.$url.']';
$func=create_function('$p_match', 'if( 0 === preg_match( "/([[:alpha:]][-+.[:alnum:]]*):\/\//", $p_match[0] ) ) {
return \'\'. $p_match[0] . \'\';
} else {
return $p_match[0];
}' );
var_dump( preg_replace_callback( $regex, $func, $str) );

returns NULL

echo preg_last_error();

returns 2 (== PREG_BACKTRACK_LIMIT_ERROR)

</pre>

So in the end it's not related to PHP or to a MantisBT bug, but rather due to a memory issue. According to PHP documentation [1], the default value for this is 1'000'000 and 100'000 for PHP < 5.3.7.

You should update your php.ini settings - or use shorter URL's ;-)

[1] http://us3.php.net/manual/en/pcre.configuration.php#ini.pcre.backtrack-limit

EDIT: breaking long lines

Henrik Morgenbrodt

Henrik Morgenbrodt

2012-07-04 03:07

reporter   ~0032241

Last edited: 2012-07-04 03:15

Confirmed; was able to solve it with
<pre>ini_set('pcre.backtrack_limit', max(10000000,ini_get('pcre.backtrack_limit')));</pre>

FYI: For this project PHP 5.3.3 was used.

Thanks a lot for your support!

UPDATE: Replaced <pre>ini_set('pcre.backtrack_limit', 10000000);</pre> to make sure the value is not lowered if raised elsewhere.

atrol

atrol

2012-07-04 03:11

developer   ~0032242

@dregad
Maybe something for the trouble shooting chapter in admin guide and/or soemthing we should check in admin/check.php?

dregad

dregad

2012-07-04 03:33

developer   ~0032243

@Henrik - glad to be of assistance

@atrol - I thought about this some more last night, and believe it would be good to (somehow / optionally) report a warning, and then display the unformatted string instead of just printing the NULL value (i.e. empty string) returned by the function.

The error itself is easy to catch: both preg_replace_callback return value === NULL and preg_last_error return value != 0 allow to detect it.

The real question is how to properly handle it; it could be done

  • by throwing an application error,
  • "silently" in string_insert_hrefs, or in the plugin by catching the return value
    with the latter 2, a warning message could be pre/app-ended to the processed string to indicate that a problem occured.

Each option has pros and cons; the error would break application flow and make it difficult to fix the problem. With the other method, if silent then user may wonder why some email addresses are hyperlinked and others not; and with the warning we actually alter the data to display which could be objectionable (but I personally think that's the best option)

Thoughts ?

dregad

dregad

2012-07-04 05:03

developer   ~0032246

Last edited: 2012-07-04 12:00

Attempt at more graceful handling of this problem - see attached screenshot (long_url_warning.png) and code on github https://github.com/dregad/mantisbt/tree/fix-14447.

Note: warning message is only displayed if $g_display_errors[E_USER_WARNING] = 'inline')

Let me know your thoughts / comments

EDIT: removed the previously attached patch (had an error) and added instead the link to a github to branch also containing documentation update (troubleshooting section)

dregad

dregad

2012-07-04 12:10

developer   ~0032248

This issue was introduced by the fix for 0012781 (string_insert_hrefs did not call preg_replace_callback prior to that)

atrol

atrol

2012-07-04 15:21

developer   ~0032249

I was wondering that it took so long until a user is getting this issue.
Now it's clear 0014447:0032248

This issue was introduced by the fix for 0012781
which means only reproducible with version 1.2.11 and PHP < 5.3.7

I like your try for a clean fix, but do we need such kind of fix?
Maybe there is another way (better regex, ...) to avoid reaching backtrack_limit?

What do you think about what Henrik wrote 0014447:0032241?

ini_set('pcre.backtrack_limit', max(10000000,ini_get('pcre.backtrack_limit')));

The setting has been introduced in PHP 5.2
http://us3.php.net/manual/en/pcre.configuration.php#ini.pcre.backtrack-limit
This should work as a simpler fix in master branch, where we check for PHP >= 5.3.2 but not in master-1.2.x where we check for 5.1.0.

Maybe you want to write also to the developer mailing list where more experienced PHP developers than myself are listing.

dregad

dregad

2012-07-05 06:59

developer   ~0032253

Maybe there is another way (better regex, ...) to avoid reaching backtrack_limit?

It's possible that there is a better way to write this regex, but I can't think of one... guess I have reached my limits in regex expertise here ;-) Actually it might be better to rethink how this function operates, maybe by splitting the text in smaller pieces. I'll think about it.

What do you think about what Henrik wrote 0014447:0032241?

ini_set('pcre.backtrack_limit', max(10000000,ini_get('pcre.backtrack_limit')));

Increasing the limit is not fixing the root cause, just making it less likely to happen, therefore I think the added error handling is still useful with the current code.

Even with an increased limit, you could still hit the bug if the URL is big enough. I tested with 1'000'000 (PHP 5.3.7 and above default), and it breaks with 581 chars on my dev box; with 10'000'000 the error occurs at 1829 chars (function call is 0.2 seconds though, vs 0.001 s. with 150 chars).

The setting has been introduced in PHP 5.2 [...] in master-1.2.x where we check for 5.1.0.

Good point on PHP compatibility version here, forgot about 5.1. To maintain backwards compatibility we'd need to add some version checking to avoid errors.

We do recommend 5.2.x however, and quite frankly in 2012 it's reasonable to expect that anyone would (more like should IMO) be on 5.2 or even 5.3... Latest 5.1.x version is 5.1.6, released in August 2006, so aybe it's time to change the requirements ?

That being said, I googled a bit, and it seems [1] (no hard evidence e.g. php doc though) that preg functions in php < 5.1.x did not return NULL or an error code; instead returned the original string.

[1] http://www.pelagodesign.com/blog/2008/01/25/wtf-preg_replace-returns-null/

dregad

dregad

2012-07-06 08:35

developer   ~0032263

I believe I found a more elegant and better performing solution to this problem using preg_split() function, that does not seem to trigger any errors regardless of URL size [1]

I would appreciate if you could test it [2].

Attached tar file with old solution for future reference.

[1] Tested with >5000 chars and pcre.backtrack_limit = 100000
[2] https://github.com/dregad/mantisbt/tree/fix-14447

atrol

atrol

2012-07-06 17:12

developer   ~0032267

I tried PHP 5.3.6 with pcre.backtrack_limit 100000
I did a few tests with your branch and got no errors.

After that I tried latest master-1.2.x to be sure that my environment is able to reproduce the issue and was quite surprised that I was not able to reproduce it.
I tried 1.2.11 and was able to reproduce the issue.
Seems that your other commit [1] fixed also the issue.

[1] https://github.com/mantisbt/mantisbt/commit/6da5164e35ffa80afeb8a04dcc4b8d2fa978f172

dregad

dregad

2012-07-09 04:54

developer   ~0032273

Call it Regular Expression Voodoo Magic ;-)

So the new regex is maybe a bit more efficient, but it still breaks (at 316 chars with pcre.backtrack_limit = 100000). I think it's still worth pushing my fix.

atrol

atrol

2012-07-09 07:37

developer   ~0032276

I am no Regular Expression Voodoo Master.
Don't expect to get any wise comment from me.

dregad

dregad

2012-07-22 18:30

developer   ~0032355

Henrik, is there any chance you could test my proposed fix (see 0014447:0032263) ? Your feedback would be appreciated.

dregad

dregad

2012-08-03 04:59

developer   ~0032457

Henrik, as you have not provided any feedback I assume you are fine with the fix. I will push it to core so it's included in the next release of MantisBT, and resolve this issue.

grangeway

grangeway

2013-04-05 17:56

reporter   ~0036141

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch

Related Changesets

MantisBT: master dcc19f49

2012-07-04 03:39

dregad


Details Diff
Fix long URL formatting causing bugnote/text to be blank

When MantisCoreFormatting plugin processes text (e.g. bugnote)
containing a long URL, a PREG_BACKTRACK_LIMIT_ERROR may occur in
string_insert_hrefs() when replacing email address links with
preg_replace_callback(). When pcre.backtrack_limit = 100000, the longest
URL that can be processed successfully is 152 chars. A bigger URL
triggers the PCRE error and the function returns NULL.

This was introduced by commit 01b5bf55a1dae4225a083cca16edf208bb10c4b1.

We now use preg_split() instead of preg_replace_callback(), which is
more efficient and avoids the backtrace error issue

The url replacement string was also moved to a variable to make code
more readable.

Fixes 0014447
Affected Issues
0014447
mod - core/string_api.php Diff File

MantisBT: master-1.2.x 38ef3244

2012-07-04 03:39

dregad


Details Diff
Fix long URL formatting causing bugnote/text to be blank

When MantisCoreFormatting plugin processes text (e.g. bugnote)
containing a long URL, a PREG_BACKTRACK_LIMIT_ERROR may occur in
string_insert_hrefs() when replacing email address links with
preg_replace_callback(). When pcre.backtrack_limit = 100000, the longest
URL that can be processed successfully is 152 chars. A bigger URL
triggers the PCRE error and the function returns NULL.

This was introduced by commit 21a3469d1039c21fbf52796f23a95ab18856254a.

We now use preg_split() instead of preg_replace_callback(), which is
more efficient and avoids the backtrace error issue

The url replacement string was also moved to a variable to make code
more readable.

Fixes 0014447
Affected Issues
0014447
mod - core/string_api.php Diff File

MantisBT: master-1.2.x 19b2865e

2012-07-06 00:41

dregad


Details Diff
Store bugnote URL replacement string in a static variable

The purpose is to make code more readable.

Follow up on fix for issue 0014447.
Affected Issues
0014447
mod - core/string_api.php Diff File

MantisBT: master 71c7e027

2012-07-06 00:41

dregad


Details Diff
Store bugnote URL replacement string in a static variable

The purpose is to make code more readable.

Follow up on fix for issue 0014447.
Affected Issues
0014447
mod - core/string_api.php Diff File