View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0022028 | mantisbt | performance | public | 2016-12-09 10:19 | 2016-12-30 15:54 |
Reporter | nicolass | Assigned To | cproensa | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Target Version | 1.3.5 | Fixed in Version | 1.3.5 | ||
Summary | 0022028: Time out caused by URL processing | ||||
Description | Hi. I want to know how I cant limit the "Description" field when I'm triying to report a bug. This is because in our organization's mantis someone uploaded a WebService return and that blocked the access to the bug, it never charges, when I elevated the parameter max_execution_time in php.ini file to 300 seconds, an error appeared in the note who had the WS return: Fatal error: Maximum execution time of 300 seconds exceeded in /xxxx/xxx/mantis/core/string_api.php on line 533 I notice that the php process in my server while the page is charging load the proccessor close to 95% | ||||
Tags | No tags attached. | ||||
Attached Files | |||||
This might be caused by an old PHP version. Which version of MantisBT do you use? |
|
Thanks for your answer: We have mantis in production with:
In our test environment we have:
The "Fatal Error" is replicated in both environments but I could get the error in the test environment, in production the page never charges. BTW Sorry for my bad english, is not my native language |
|
Could you upload the description of the affected issue as an attachment? |
|
Sorry but I dont understand. Do you want the WS return that someone uploaded as a description? |
|
Right, |
|
I just uploaded it |
|
Can you confirm that you don't get any longer the timeout if you deactivate "URL Processing" in "MantisBT Formatting 1.3.0" plugin? |
|
Yes I deactivated "URL Processing" and the bug charges immediately. This on the version 1.3.4. I don't have access to admin privileges on production mantis, so, those are the same steps for the version 1.2.8? Thanks |
|
I reproduced this, and fixed by adding a length limitation to the email regex. <pre> after notice change + into {1,64} @dregad, you have more experience with these f--- regexps. any suggestion? |
|
Keep in mind: This is just a workaround as you deactivate some functionality with it, |
|
@cproensa, for the record the e-mail matching Regex we're currently using is taken (almost) verbatim from the HTML5 specification [1]. It is itself derived from the addr-spec specification, as defined in RFC 5322 (Internet message format) [2], which does not define a maximum size for the "local part" of the address (hence the W3C's choice to use '+' in the regex). That being said, RFC 5321 (SMTP) does define size limits [3] but they are recommendations not strict rules.
The local part SHOULD be <= 64 bytes, so your proposed fix matches the spec's recommendation. I think it's perfectly acceptable to modify the regex as you suggest.
Maximum length for domain SHOULD be 255 bytes.
I believe implementing that would require a rather complex regex (using positive lookahead), and TBH I'm not really sure it's worth the effort. [1] https://github.com/mantisbt/mantisbt/blob/master/core/email_api.php |
|
By limiting the local part, it will match a substring if the string is longer. This may not be desirable, i'd say the ideal is to not match that |
|
True, but to avoid that we'd need to add a "lookbehind" group to the regex, to make sure the previous char is not a valid word char, something like this: Test string (with regex modified to limit 5 per your earlier example): Of course the regex is a lot less efficient like this (about 50% more steps required / 30% slower to complete the match), so using the attached bugnote as a test case we get:
(Tested with https://regex101.com/)
In any case it's better for sure , since it prevents the regex from timing out / out of memory... Now the question is whether the performance tradeoff is truly necessary, considering that the size limit we already have in the domain name matching group already causes a similar behavior: 'abcdefgh@<insert 61 chars here>xxx' to match up to and not including the 'xxx'. |
|
Thanks for the link. I have learned a lot of these ? syntax... So, should be this the correct one? (redacted as logical groups) lookbehind for a non-local char: With the same test case, it returns 992738 steps, 2 matches Full regex: |
|
Yes, definitely the best regex resource I've found, both for developing, debugging and educational purposes. Did you try the debugger and unit tests tools ?
I don't have time to look in detail atm, but I'm not sure what you're trying to achieve with that bit. In particular, where is the 254 limit coming from ? IIRC there's a 255 limit on the domain, not on the whole address. All regex fun & games aside, let me ask again: is the performance hit caused by a complex regex to avoid a "false match" truly necessary ? |
|
basically, limiting the match group length. When a valid char appears, and there are more than MAX valid chars ahead, this should fail.
With long "rubbish" text that may cause false matches due some appearances of "@", having long strings formatted as email links is not pretty. so, truly necessary? no, as shorter matches within the allowed limits would still appear as false positives. Do you know a way to check for the 254 lookahead and shortcut a fail, or discard that part right away? probably that would be lighter. |
|
PR: https://github.com/mantisbt/mantisbt/pull/978 |
|
MantisBT: master-1.3.x 31c7ecdd 2016-12-16 07:59 Committer: dregad Details Diff |
Add limits to email regex to avoid timeouts on very long texts Adds a limit of 64 chars on the local part of the email address, as defined by: https://tools.ietf.org/html/rfc5321#section-4.5.3.1 This avoids timeout errors on very long texts with false matches. Fixes: 0022028 |
Affected Issues 0022028 |
|
mod - core/email_api.php | Diff File |