Redmine

I have stumbled upon a very nasty bug with journal entries quoting, when a user writes an entry with a "pre" tag but forgets to put a proper "/pre" tag.

When trying to quote such an entry where a "pre" tag is not closed properly, the regexp engine can go haywire (process stuck in gsub() forever, CPU usage 100%, if it happens with all threads of an existing instance, the whole Redmine instance becomes unusable until the process finishes/is terminated) with cases where a lot of whitespaces/tabs/newlines are present.

Upon stumbling on that bug, and killing the affected Rails processes, I got the following trace :

Completed 500 Internal Server Error in 7016997ms
SignalException (SIGTERM):
  app/controllers/journals_controller.rb:69:in `gsub'
  app/controllers/journals_controller.rb:69:in `new'

Which allowed me to narrow it down the code where "pre" blocks are replaced with "[...]".

Studying the ruby regular expression spec (http://www.tutorialspoint.com/ruby/ruby_regular_expressions.htm) it seems the following expression

%r{<pre>((.|\s)*?)</pre>}m

has redundant elements : a "dot" is supposed to match anything including newlines, when the regexp has the 'm' flag active, so there is no purpose in having a "or" match with whitespaces.

I tried the following fix on my side (basically, removing the redundant "or \s" check and sticking with "." and the 'm' flag) :

--- /usr/local/share/redmine/app/controllers/journals_controller.rb    2013-05-02 00:15:29.000000000 +0900
+++ /tmp/journals_controller.rb    2014-03-14 18:35:24.498448622 +0900
@@ -66,7 +66,7 @@
       text = @issue.description
     end
     # Replaces pre blocks with [...]
-    text = text.to_s.strip.gsub(%r{<pre>((.|\s)*?)</pre>}m, '[...]')
+    text = text.to_s.strip.gsub(%r{<pre>(.*?)</pre>}m, '[...]')
     @content = "#{ll(Setting.default_language, :text_user_wrote, user)}\n> " 
     @content << text.gsub(/(\r?\n|\r\n?)/, "\n> ") + "\n\n" 
   rescue ActiveRecord::RecordNotFound

and it seems to fix the issue.

How to reproduce/check (it "works" in every case, as in, it WILL eventually end, as it is not an infinite loop, but the execution time varies greatly) :

This works (obviously) :

$ time ruby -e 'print "<pre>  lalala       \tlalala</pre>".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')' 
[...]
real    0m0.015s
user    0m0.011s
sys    0m0.004s
$ time ruby -e 'print "<pre>  lalala       \tlalala\n     lalala\r\n   lala\r    \n        lalalla \t\n</pre>".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')' 
[...]
real    0m0.018s
user    0m0.014s
sys    0m0.005s

I apologize in advance for the display break, but there is no way I can provide proper sample code for this problem otherwise... (I hope this does not trigger anything weird by writing it in the description, if this has not been fixed in latest versions...)

These take more and more time as you add more characters, rendering a whole running ruby process unusable :

# Starting with 20 characters, mixing spaces and alphabetic
$ time ruby -e 'print "<pre>  lalala       ".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')' 
<pre>  lalala       
real    0m0.069s
user    0m0.007s
sys    0m0.004s
# Adding 7 more characters (total : 27)
$ time ruby -e 'print "<pre>  lalala       \tlalalal".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')' 
<pre>  lalala           lalalal
real    0m0.018s
user    0m0.013s
sys    0m0.008s
# Adding 14 more characters (total : 41)
$ time ruby -e 'print "<pre>  lalala        \tlalala\n     lalala\r\n".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')' 
<pre>  lalala            lalala
     lalala

real    0m0.080s
user    0m0.080s
sys    0m0.001s
# Adding 3 more characters (total : 44)
$ time ruby -e 'print "<pre>  lalala        \tlalala\n     lalala\r\n   ".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')' 
<pre>  lalala            lalala
     lalala

real    0m0.349s
user    0m0.344s
sys    0m0.004s
# Adding 4 more characters (total : 48)
$ time ruby -e 'print "<pre>  lalala        \tlalala\n     lalala\r\n   lala".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')' 
<pre>  lalala            lalala
     lalala
   lala
real    0m1.010s
user    0m1.009s
sys    0m0.001s
# Adding 5 more characters (total : 53)
$ time ruby -e 'print "<pre>  lalala       \tlalala\n     lalala\r\n   lala\r    ".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')' 
<pre>  lalala           lalala
     lalala
    ala
real    0m5.213s
user    0m5.208s
sys    0m0.001s
# Adding 5 more characters (total : 58)
$ time ruby -e 'print "<pre>  lalala       \tlalala\n     lalala\r\n   lala\r    \n    ".gsub(%r{<pre>((.|\s)*?)</pre>}m, '"'[...]'"')'
<pre>  lalala           lalala
     lalala
    ala

real    2m36.084s
user    2m36.021s
sys    0m0.028s
</pre>
...And it only gets worse from there.

This works, and ends instantly :
<pre>
$ time ruby -e 'print "<pre>  lalala       \tlalala\n     lalala\r\n   lala\r    \n        lalalla \t\n".gsub(%r{<pre>(.*?)</pre>}m, '"'[...]'"')' 
<pre>  lalala           lalala
     lalala
    ala
        lalalla     

real    0m0.014s
user    0m0.014s
sys    0m0.001s
$ time ruby -e 'print "<pre>  lalala       \tlalala\n     lalala\r\n   lala\r    \n        lalalla \t\n</pre>".gsub(%r{<pre>(.*?)</pre>}m, '"'[...]'"')' 
[...]
real    0m0.018s
user    0m0.014s
sys    0m0.004s
</pre>