View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0014447 | mantisbt | other | public | 2012-07-03 09:20 | 2014-09-23 18:05 |
Reporter | Henrik Morgenbrodt | Assigned To | dregad | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
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. 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. 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 | ||||
Tags | No tags attached. | ||||
Attached Files | |||||
related to | 0012781 | closed | dregad | Links in the comments look broken |
related to | 0015721 | closed | grangeway | Functionality to consider porting to master-2.0.x |
has duplicate | 0014654 | closed | dregad | Bugnotes disappear when contains a very long link |
has duplicate | 0015202 | closed | dregad | string_insert_hrefs() returns NULL in case of long URLs in text |
has duplicate | 0015462 | closed | atrol | At times, content in text fields in a ticket is hidden completely when ticket is read-only |
related to | 0014547 | closed | dregad | Email hrefs in bugnotes should include the 'mailto:' prefix |
I was right. the bugnote before seems to be empty. |
|
This is caused by function string_insert_hrefs which is called if "URL Processing" is enabled (the default) in standard plugin MantisCoreFormatting |
|
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 |
|
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> |
|
Reproducing issue outside of mantisbt on my box: <pre> returns NULLecho 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 |
|
Confirmed; was able to solve it with 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. |
|
@dregad |
|
@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
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 ? |
|
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) |
|
This issue was introduced by the fix for 0012781 (string_insert_hrefs did not call preg_replace_callback prior to that) |
|
I was wondering that it took so long until a user is getting this issue.
I like your try for a clean fix, but do we need such kind of fix? What do you think about what Henrik wrote 0014447:0032241?
The setting has been introduced in PHP 5.2 Maybe you want to write also to the developer mailing list where more experienced PHP developers than myself are listing. |
|
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.
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).
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/ |
|
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 |
|
I tried PHP 5.3.6 with pcre.backtrack_limit 100000 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. [1] https://github.com/mantisbt/mantisbt/commit/6da5164e35ffa80afeb8a04dcc4b8d2fa978f172 |
|
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. |
|
I am no Regular Expression Voodoo Master. |
|
Henrik, is there any chance you could test my proposed fix (see 0014447:0032263) ? Your feedback would be appreciated. |
|
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. |
|
Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch |
|
MantisBT: master dcc19f49 2012-07-04 03:39 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 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 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 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 |