samizdat-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug #19903: focus/resource/message_helper recursion in 0.6.0.20070509


From: boud
Subject: bug #19903: focus/resource/message_helper recursion in 0.6.0.20070509
Date: Fri, 18 May 2007 23:11:24 +0200 (CEST)

hi samizdat-devel,

i posted this on savannah.nongnu:
  https://savannah.nongnu.org/bugs/index.php?19903
but the default format is not very nice so here it is with indenting.

For obvious reasons, i suggest that this bug fix or an equivalent one
go into cvs reasonably quickly.


BUG: severe

VERSION: 0.6.0.20070509-1

PROBLEM: recursion and crash if message A is related to message B and
message B is related to message A. This occurred on a live site! The error
causes a crash (with error hash given to the user) after the second relation
is voted on, or when trying to load the frontpage.

The recursion probably did not exist in pre-20070501, since the recursive
A->B->A relationship existed in the database without causing errors in the pages rendered through apache pre-20070501.



DIAGNOSIS:
Key lines excerpted from the three routines involved in the loop.

models/focus.rb:
class Focus
  def initialize(request, id=nil, related=nil)   # todo: unit-test this beast
    if id   # existing resource
      @resource = Resource.new(request, id)

components/resource.rb:
class Resource
  def initialize(request, id)
    @component = instance_eval(@type + 'Component.new(@request, @id)')

class MessageComponent < ResourceComponent
  def initialize(request, id)
    @info = message_info(@message, :list)

helpers/message_helper.rb:
module MessageHelper
  def message_info(message, mode)
    focuses = focus_line(message.id, message.focuses.collect {|f|
      Focus.new(@request, f, message.id)
    })


PROPOSED SOLUTION:
An anti-recursion counter could probably be placed in any of these three
files. MessageHelper::message_info seemed a good place, in order to avoid intervening in the more central parts of the whole system. The patch
is below.


cheers
boud


--- /tmp/s070509/samizdat/lib/samizdat/helpers/message_helper.rb        
2007-04-30 23:46:20.000000000 +0200
+++ /usr/lib/ruby/1.8/samizdat/helpers/message_helper.rb        2007-05-18 
22:37:46.216839296 +0200
@@ -13,6 +13,8 @@
 module MessageHelper
   include ApplicationHelper

+  @@recursion_depth = 0
+
   # render link to full message
   #
   def full_message_href(id)
@@ -37,6 +39,8 @@

   # render _message_ info line (except focuses)
   #
+#  def message_info(message, mode)
+
   def message_info(message, mode)
     creator = message.creator.id.nil? ?   # no link if published by guest
       _('guest') : %{<a 
href="#{message.creator.id}">#{message.creator.full_name}</a>}
@@ -61,9 +65,18 @@
       message.translations.sort_by {|l,m,r| -r.to_f }.collect {|l,m,r|
         %{<a href="#{m}">#{l}</a>}
       }.join(' ') if message.translations.to_a.size > 0
-    focuses = focus_line(message.id, message.focuses.collect {|f|
-      Focus.new(@request, f, message.id)
-    })
+
+
+    @@recursion_depth += 1
+    if @@recursion_depth <= 1
+      focuses = focus_line(message.id, message.focuses.collect {|f|
+                             Focus.new(@request, f, message.id)
+                           })
+    else
+      focuses = nil
+    end
+    @@recursion_depth = 0
+
     hidden = _('hidden') if message.hidden?

     [ sprintf(_('by&nbsp;%s on&nbsp;%s'), creator, date.to_s),
@@ -181,7 +194,9 @@
       title = Focus.focus_title(title) if
         :short == mode and message.nrelated > 0
       title = CGI.escapeHTML(limit_string(title))
-      title = %{<div class="title">#{resource_href(message.id, title)}</div>\n}
+      trans = (config['separate_translations'])?
+        message.select_translation(@request.accept_language) : message
+      title = %{<div class="title">#{resource_href(trans.id, title)}</div>\n}
     end
     if translation.desc and translation.desc != translation.id
       # use description of translation when applicable
kazuyo:/tmp#
kazuyo:/tmp# fg
emacs21 -nw /usr/lib/ruby/1.8/samizdat/helpers/sms2message.rb

[1]+  Stopped                 emacs21 -nw 
/usr/lib/ruby/1.8/samizdat/helpers/sms2message.rbkazuyo:/tmp#
kazuyo:/tmp#
kazuyo:/tmp#
kazuyo:/tmp# diff -u 
/tmp/s070509/samizdat/lib/samizdat/helpers/message_helper.rb 
/usr/lib/ruby/1.8/samizdat/helpers/message_helper.rb
--- /tmp/s070509/samizdat/lib/samizdat/helpers/message_helper.rb        
2007-04-30 23:46:20.000000000 +0200
+++ /usr/lib/ruby/1.8/samizdat/helpers/message_helper.rb        2007-05-18 
22:44:35.016692272 +0200
@@ -13,6 +13,8 @@
 module MessageHelper
   include ApplicationHelper

+  @@recursion_depth = 0
+
   # render link to full message
   #
   def full_message_href(id)
@@ -61,9 +63,18 @@
       message.translations.sort_by {|l,m,r| -r.to_f }.collect {|l,m,r|
         %{<a href="#{m}">#{l}</a>}
       }.join(' ') if message.translations.to_a.size > 0
-    focuses = focus_line(message.id, message.focuses.collect {|f|
-      Focus.new(@request, f, message.id)
-    })
+
+
+    @@recursion_depth += 1
+    if @@recursion_depth <= 1
+      focuses = focus_line(message.id, message.focuses.collect {|f|
+                             Focus.new(@request, f, message.id)
+                           })
+    else
+      focuses = nil
+    end
+    @@recursion_depth = 0
+
     hidden = _('hidden') if message.hidden?

     [ sprintf(_('by&nbsp;%s on&nbsp;%s'), creator, date.to_s),


######################################################################




reply via email to

[Prev in Thread] Current Thread [Next in Thread]