MantisBT

View Issue Details Jump to Notes ] Wiki ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0014447mantisbtotherpublic2012-07-03 09:202013-04-06 09:23
ReporterHenrik Morgenbrodt 
Assigned Todregad 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version1.2.11 
Target Version1.2.12Fixed in Version1.2.12 
Summary0014447: URLs longer than 152 characters are causing problems
DescriptionUsers 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 ReproduceTo 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 [^]
Tags2.0.x check
Attached Filespng file icon long_url_warning.png [^] (14,054 bytes) 2012-07-04 04:56


gz file icon fix-14447-preg_replace_callback-catch-error.tar.gz [^] (2,788 bytes) 2012-07-06 10:50

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

-  Notes
User avatar (0032231)
Henrik Morgenbrodt (reporter)
2012-07-03 09:20
edited on: 2012-07-06 16:53

http://esolut.de/somelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsom [^]

User avatar (0032232)
Henrik Morgenbrodt (reporter)
2012-07-03 09:21

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.
User avatar (0032234)
atrol (developer)
2012-07-03 10:28

This is caused by function string_insert_hrefs which is called if "URL Processing" is enabled (the default) in standard plugin MantisCoreFormatting
User avatar (0032235)
dregad (developer)
2012-07-03 10:36
edited on: 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

User avatar (0032236)
dregad (developer)
2012-07-03 12:06

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
    $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 );
User avatar (0032237)
dregad (developer)
2012-07-03 12:23
edited on: 2012-07-03 12:28

Reproducing issue outside of mantisbt on my box:
$regex="/(([[:alpha:]][-+.[:alnum:]]*):\/\/.*?)?([a-z0-9!#*+\/=?^_{|}~-]+"
. "(?:\.[a-z0-9!#*+\/=?^_{|}~-]+)*)"
. "\@((?:[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)


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

User avatar (0032241)
Henrik Morgenbrodt (reporter)
2012-07-04 03:07
edited on: 2012-07-04 03:15

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


FYI: For this project PHP 5.3.3 was used.

Thanks a lot for your support!

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

User avatar (0032242)
atrol (developer)
2012-07-04 03:11

@dregad
Maybe something for the trouble shooting chapter in admin guide and/or soemthing we should check in admin/check.php?
User avatar (0032243)
dregad (developer)
2012-07-04 03:33

@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 ?
User avatar (0032246)
dregad (developer)
2012-07-04 05:03
edited on: 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)

User avatar (0032248)
dregad (developer)
2012-07-04 12:10

This issue was introduced by the fix for 0012781 (string_insert_hrefs did not call preg_replace_callback prior to that)
User avatar (0032249)
atrol (developer)
2012-07-04 15:21

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.
User avatar (0032253)
dregad (developer)
2012-07-05 06:59

> 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/ [^]
User avatar (0032263)
dregad (developer)
2012-07-06 08:35

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 [^]
User avatar (0032267)
atrol (developer)
2012-07-06 17:12

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 [^]
User avatar (0032273)
dregad (developer)
2012-07-09 04:54

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.
User avatar (0032276)
atrol (developer)
2012-07-09 07:37

I am no Regular Expression Voodoo Master.
Don't expect to get any wise comment from me.
User avatar (0032355)
dregad (developer)
2012-07-22 18:30

Henrik, is there any chance you could test my proposed fix (see 0014447:0032263) ? Your feedback would be appreciated.
User avatar (0032457)
dregad (developer)
2012-08-03 04:59

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.
User avatar (0036141)
grangeway (developer)
2013-04-05 17:56

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

- Related Changesets
MantisBT: master dcc19f49
Timestamp: 2012-07-04 07:39:23
Author: 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
mod - core/string_api.php Diff ] File ]
MantisBT: master-1.2.x 38ef3244
Timestamp: 2012-07-04 07:39:23
Author: 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
mod - core/string_api.php Diff ] File ]
MantisBT: master-1.2.x 19b2865e
Timestamp: 2012-07-06 04:41:24
Author: 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.
mod - core/string_api.php Diff ] File ]
MantisBT: master 71c7e027
Timestamp: 2012-07-06 04:41:24
Author: 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.
mod - core/string_api.php Diff ] File ]

- Issue History
Date Modified Username Field Change
2012-07-03 09:20 Henrik Morgenbrodt New Issue
2012-07-03 09:20 Henrik Morgenbrodt Note Added: 0032231
2012-07-03 09:21 Henrik Morgenbrodt Note Added: 0032232
2012-07-03 10:23 dregad Note Added: 0032233
2012-07-03 10:24 dregad Note Edited: 0032233 View Revisions
2012-07-03 10:28 atrol Note Added: 0032234
2012-07-03 10:36 dregad Note Added: 0032235
2012-07-03 10:49 dregad Note Edited: 0032235 View Revisions
2012-07-03 10:52 dregad Note Deleted: 0032233
2012-07-03 12:06 dregad Note Added: 0032236
2012-07-03 12:23 dregad Note Added: 0032237
2012-07-03 12:23 dregad Status new => resolved
2012-07-03 12:23 dregad Resolution open => no change required
2012-07-03 12:23 dregad Assigned To => dregad
2012-07-03 12:26 dregad Note Edited: 0032237 View Revisions
2012-07-03 12:27 dregad Note Edited: 0032237 View Revisions
2012-07-03 12:28 dregad Note Edited: 0032237 View Revisions
2012-07-04 03:07 Henrik Morgenbrodt Note Added: 0032241
2012-07-04 03:11 atrol Note Added: 0032242
2012-07-04 03:13 Henrik Morgenbrodt Note Edited: 0032241 View Revisions
2012-07-04 03:15 Henrik Morgenbrodt Note Edited: 0032241 View Revisions
2012-07-04 03:33 dregad Note Added: 0032243
2012-07-04 04:56 dregad File Added: long_url_warning.png
2012-07-04 05:03 dregad File Added: fix-14447.patch
2012-07-04 05:03 dregad Note Added: 0032246
2012-07-04 05:04 dregad Status resolved => feedback
2012-07-04 05:04 dregad Resolution no change required => reopened
2012-07-04 05:07 dregad Note Edited: 0032246 View Revisions
2012-07-04 12:00 dregad Note Edited: 0032246 View Revisions
2012-07-04 12:09 dregad Relationship added related to 0012781
2012-07-04 12:10 dregad Note Added: 0032248
2012-07-04 15:21 atrol Note Added: 0032249
2012-07-05 06:59 dregad Note Added: 0032253
2012-07-06 08:35 dregad Note Added: 0032263
2012-07-06 08:36 dregad File Deleted: fix-14447.patch
2012-07-06 08:36 dregad File Added: fix-14447-preg_replace_callback-catch-error.tar-gz
2012-07-06 10:50 dregad File Added: fix-14447-preg_replace_callback-catch-error.tar.gz
2012-07-06 10:50 dregad File Deleted: fix-14447-preg_replace_callback-catch-error.tar-gz
2012-07-06 16:53 atrol Note Edited: 0032231 View Revisions
2012-07-06 17:12 atrol Note Added: 0032267
2012-07-09 04:54 dregad Note Added: 0032273
2012-07-09 07:37 atrol Note Added: 0032276
2012-07-22 18:30 dregad Note Added: 0032355
2012-08-03 04:23 dregad Relationship added related to 0014547
2012-08-03 04:59 dregad Changeset attached => MantisBT master dcc19f49
2012-08-03 04:59 dregad Status feedback => resolved
2012-08-03 04:59 dregad Fixed in Version => 1.3.x
2012-08-03 04:59 dregad Changeset attached => MantisBT master-1.2.x 38ef3244
2012-08-03 04:59 dregad Note Added: 0032457
2012-08-03 04:59 dregad Resolution reopened => fixed
2012-08-03 04:59 dregad Fixed in Version 1.3.x => 1.2.12
2012-08-03 04:59 dregad Target Version => 1.2.12
2012-08-17 06:27 dregad Changeset attached => MantisBT master-1.2.x 19b2865e
2012-08-17 06:27 dregad Changeset attached => MantisBT master 71c7e027
2012-08-27 08:31 dregad Relationship added has duplicate 0014654
2012-11-10 18:54 dregad Status resolved => closed
2012-11-12 11:01 dregad Relationship added related to 0015202
2012-11-12 13:30 dregad Relationship replaced has duplicate 0015202
2013-02-05 16:20 atrol Relationship added has duplicate 0015462
2013-04-05 17:56 grangeway Status closed => acknowledged
2013-04-05 17:56 grangeway Note Added: 0036141
2013-04-05 19:30 grangeway Relationship added related to 0015721
2013-04-06 03:40 dregad Status acknowledged => closed
2013-04-06 07:23 grangeway Status closed => acknowledged
2013-04-06 09:22 dregad Tag Attached: 2.0.x check
2013-04-06 09:23 dregad Status acknowledged => closed


MantisBT 1.2.16dev master-1.2.x-8c2bd07 [^]
Copyright © 2000 - 2013 MantisBT Team
Time: 0.1462 seconds.
memory usage: 3,047 KB
Powered by Mantis Bugtracker