List Info

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




bug #19903: focus/resource/message_helper recursion in 0.6.0.20070509
user name
2007-05-18 16:11:24
hi samizdat-devel,

i posted this on savannah.nongnu:
   http
s://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="#">#</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="#">#</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),


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


_______________________________________________
samizdat-devel mailing list
samizdat-develnongnu.org
http://lists.nongnu.org/mailman/listinfo/samizdat-devel

Re: bug #19903: focus/resource/message_helper recursion in 0.6.0.20070509
user name
2007-05-19 07:57:25
On 5/18/07, boud <boudriseup.net> wrote:
> hi samizdat-devel,
>
> i posted this on savannah.nongnu:
>    http
s://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.

Done. I decided to fix this in MessageComponent: when we
are
initializing a ResourceComponent just for the title (as is
done in
Focus), we don't want the constructor to do expensive and
recursion-prone operations such as message_info(), so I
moved it out
of constructor to an on-demand call.

A package including this fix, monkey fix to sync.rb (looks
like
upstream is in no rush to include this fix), and config
fixes, will
soon appear in experimental.

-- 
Dmitry Borodaenko


_______________________________________________
samizdat-devel mailing list
samizdat-develnongnu.org
http://lists.nongnu.org/mailman/listinfo/samizdat-devel

[1-2]

about | contact  Other archives ( Real Estate discussion Medical topics )