| Anonymous | Login | Signup for a new account | 2013-05-19 12:44 EDT | ![]() |
| Main | My View | View Issues | Change Log | Roadmap | Wiki | ManTweet | Repositories |
| View Issue Details [ Jump to Notes ] [ Wiki ] [ Related Changesets ] | [ Issue History ] [ Print ] | ||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | ||||
| 0014447 | mantisbt | other | public | 2012-07-03 09:20 | 2013-04-06 09:23 | ||||
| Reporter | Henrik Morgenbrodt | ||||||||
| Assigned To | dregad | ||||||||
| Priority | normal | Severity | minor | Reproducibility | always | ||||
| Status | closed | Resolution | fixed | ||||||
| Platform | OS | OS Version | |||||||
| Product Version | 1.2.11 | ||||||||
| Target Version | 1.2.12 | Fixed in Version | 1.2.12 | ||||||
| Summary | 0014447: 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 [^] | ||||||||
| Tags | 2.0.x check | ||||||||
| Attached Files | |||||||||
Relationships |
|||||||||||||||||||||||||||||||
|
|||||||||||||||||||||||||||||||
Notes |
|
|
Henrik Morgenbrodt (reporter) 2012-07-03 09:20 edited on: 2012-07-06 16:53 |
http://esolut.de/somelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsomelongbutinvalidpathsom [^] |
|
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. |
|
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 |
|
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 |
|
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 );
|
|
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 |
|
Henrik Morgenbrodt (reporter) 2012-07-04 03:07 edited on: 2012-07-04 03:15 |
Confirmed; was able to solve it withini_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. |
|
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? |
|
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 ? |
|
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) |
|
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) |
|
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. |
|
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/ [^] |
|
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 [^] |
|
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 [^] |
|
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. |
|
atrol (developer) 2012-07-09 07:37 |
I am no Regular Expression Voodoo Master. Don't expect to get any wise comment from me. |
|
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. |
|
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. |
|
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 |