Improved rendering by html_escaping all strings by default.

zena · Sep 5, 2012 · a68ffb4 · a68ffb4
1 parent 0358b42
commit a68ffb4
Show file tree

Hide file tree

Showing 31 changed files with 111 additions and 84 deletions.
diff --git a/app/views/nodes/render_error.rhtml b/app/views/nodes/render_error.rhtml
@@ -0,0 +1,15 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
+  "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+
+<html lang='en' xmlns='http://www.w3.org/1999/xhtml'>
+<head>
+  <title>500 Error</title>
+</head>
+
+<body style='font-family:Helvetica, Arial, sans-serif; background:#eee;'>
+  <!-- The zena_error500 is used we running automated tests. Do not change -->
+  <div id='zena_error500' style='display:table; margin:50px auto; background:#aaa; height:200px; width:600px; border:1px solid black; border-radius:8px;'>
+  <%= @render_error %>
+  </div>
+</body>
+</html>
diff --git a/app/views/templates/document_create_tabs/_file.rhtml b/app/views/templates/document_create_tabs/_file.rhtml
@@ -1,7 +1,7 @@
 <%= upload_form_tag(:action => 'upload') %>
 <%= hidden_field 'node', 'parent_id', :value=>@node.parent_zip  %>
 <% [:redir, :js, :reload].each do |p|; next unless params[p] %>
-<input type='hidden' name='<%= p %>' value='<%= h params[p] %>'/>
+<input type='hidden' name='<%= p %>' value='<%=h params[p] %>'/>
 <% end %>
 <p class="btn_validate"><input type="submit" value='<%= _('validate') %>'/></p>
 

diff --git a/app/views/templates/document_create_tabs/_import.rhtml b/app/views/templates/document_create_tabs/_import.rhtml
@@ -1,7 +1,7 @@
 <%= upload_form_tag( :controller => 'nodes', :action => 'import', :id => @node.parent_zip ) %>
 <p class="btn_validate"><input type="submit" value='<%= _('validate') %>'/></p>
 <% [:redir, :js, :reload].each do |p|; next unless params[p] %>
-<input type='hidden' name='<%= p %>' value='<%= h params[p] %>'/>
+<input type='hidden' name='<%= p %>' value='<%=h params[p] %>'/>
 <% end %>
 
 <%= upload_field %>

diff --git a/app/views/templates/document_create_tabs/_template.rhtml b/app/views/templates/document_create_tabs/_template.rhtml
@@ -2,7 +2,7 @@
 <%= hidden_field 'node', 'parent_id', :value => @node.parent_zip  %>
 <p class="btn_validate"><input type="submit" onclick="$('loader').style.visibility = 'visible';" value='<%= _('validate') %>'/></p>
 <% [:redir, :js, :reload].each do |p|; next unless params[p] %>
-<input type='hidden' name='<%= p %>' value='<%= h params[p] %>'/>
+<input type='hidden' name='<%= p %>' value='<%=h params[p] %>'/>
 <% end %>
 
 <input type='hidden' name='node[klass]' value='Template'/>

diff --git a/app/views/templates/document_create_tabs/_text_document.rhtml b/app/views/templates/document_create_tabs/_text_document.rhtml
@@ -2,7 +2,7 @@
 <%= hidden_field 'node', 'parent_id', :value=>@node.parent_zip  %>
 <p class="btn_validate"><input type="submit" onclick="$('loader').style.visibility = 'visible';" value='<%= _('validate') %>'/></p>
 <% [:redir, :js, :reload].each do |p|; next unless params[p] %>
-<input type='hidden' name='<%= p %>' value='<%= h params[p] %>'/>
+<input type='hidden' name='<%= p %>' value='<%=h params[p] %>'/>
 <% end %>
 
 <label for='node_content_type'><%= _('content type') %></label>

diff --git a/bricks/pdf/lib/bricks/pdf.rb b/bricks/pdf/lib/bricks/pdf.rb
@@ -107,7 +107,7 @@ def render_to_pdf(opts)
             :type        => 'text/html',
             # Compile html to pdf
             :disposition => 'inline',
-            :data        => "Could not render pdf file... <a href='#{debug_url}'>debug</a>"
+            :error       => "Could not render pdf file... <a href='#{debug_url}'>debug</a>"
           }
         end
       end

diff --git a/lib/zafu/process/ruby_less_processing.rb b/lib/zafu/process/ruby_less_processing.rb
@@ -240,12 +240,14 @@ def method_with_arguments(method, params)
         def rubyless_expand(res)
           if res.klass == String && !@blocks.detect {|b| !b.kind_of?(String)}
             if lit = res.literal
-              out erb_escape(lit)
+              out param(:h) == 'false' ? erb_escape(lit) : ::ERB::Util.html_escape(lit)
             # TODO: Enable this when we have time to ensure tests/functionality work correctly.
             #elsif res.opts[:h]
             #  show_string(res)
-            else
+            elsif res.opts[:html_safe]
               out "<%= #{res} %>"
+            else
+              show_string(res)
             end
           elsif res.klass == Boolean
             expand_if(res)

diff --git a/lib/zena/core_ext/string.rb b/lib/zena/core_ext/string.rb
@@ -100,9 +100,10 @@ def abs_path(root)
   # of the readmore argument.
   def limit(size, readmore = '…')
     if self.size > size
-      self[0..(size-1)] + readmore
+      # readmore can contain a link: <a...> but this is defined in the zafu template.
+      ::ERB::Util.html_escape(self[0..(size-1)]) + readmore
     else
-      self
+      ::ERB::Util.html_escape(self)
     end
   end
 

diff --git a/lib/zena/deploy.rb b/lib/zena/deploy.rb
@@ -94,7 +94,7 @@ def ancestry(path)
       'current/log',
       'shared/log',
       # Passenger uses the owner of environment.rb as worker
-      'config/environment.rb',
+      'current/config/environment.rb',
     ].map {|dir| "#{deploy_to}/#{dir}"}
 
     # make sure production.log is created before so that it gets the correct permission
@@ -159,6 +159,11 @@ def ancestry(path)
     logrotate
     run "chown -R www-data:www-data #{sites_root}/#{self[:host]}"
   end
+
+  task :mksymlinks, :roles => :app do
+    run "#{in_current} rake zena:mksymlinks HOST='#{self[:host]}'"
+    run "chown -R www-data:www-data #{sites_root}/#{self[:host]}"
+  end
 
   desc "update code in the current version"
   task :up, :roles => :app do
@@ -230,7 +235,7 @@ def ancestry(path)
       run "test -e /etc/apache2/sites-enabled/#{self[:host]} || a2ensite #{self[:host]}" if debian_host
 
       unless self[:host] =~ /^www/
-        vhost_www = render("#{templates}/vhost_www.rhtml", :config => self, :vhost_port => (self[:ssl] ? ':80' : '80'))
+        vhost_www = render("#{templates}/vhost_www.rhtml", :config => self, :vhost_port => (self[:ssl] ? ':80' : ''))
         put(vhost_www, "#{vhost_root}/www.#{self[:host]}")
         run "test -e /etc/apache2/sites-enabled/www.#{self[:host]} || a2ensite www.#{self[:host]}" if debian_host
       end
@@ -287,7 +292,7 @@ def ancestry(path)
       else
         # create logrotate config file
         logrotate_conf = render("#{templates}/logrotate_host.rhtml", :config => self )
-        put(logrotate_conf, "/etc/logrotate.d/#{self[:host]}")
+        put(logrotate_conf, "/etc/logrotate.d/#{db_name}-#{self[:host]}")
       end
     end
   end

diff --git a/lib/zena/use/display.rb b/lib/zena/use/display.rb
@@ -432,6 +432,7 @@ def r_zazen(signature = nil)
                 :class  => String,
                 :method => 'zazen',
                 :accept_nil => true,
+                :html_safe => true,
                 :append_hash => {:node => ::RubyLess::TypedString.new(node.to_s, :class => node.klass)}
               }
             else
@@ -517,7 +518,7 @@ def extract_label(res, attribute)
 
         # ??? <r:h do='foasfa'/> ?
         # def r_h
-        #   out "<%= h ??? %>"
+        #   out "<%=h ??? %>"
         # end
 
         # Insert javascript asset tags
@@ -701,7 +702,7 @@ def show_string(method)
           if param(:h) == 'false'
             "<%= #{method} %>"
           else
-            "<%= h #{method} %>"
+            "<%=h #{method} %>"
           end
         end
 

diff --git a/lib/zena/use/html_tags.rb b/lib/zena/use/html_tags.rb
@@ -163,19 +163,19 @@ module ViewMethods
         include LinkTags
         include RubyLess
 
-        safe_method [:flash_messages] => String
+        safe_method [:flash_messages] => {:class => String, :method => 'flash_messages', :html_safe => true}
         # TODO: replace 'flash_messages' with a FlashHash context or a list
         # of Flash messages.
 
         def flash_messages(opts={})
           type = opts[:show] || 'both'
 
           if (type == 'notice' || type == 'both') && flash[:notice]
-            notice = "<div class='auto_fade notice' onclick='new Effect.Fade(this)'>#{flash[:notice]}</div>"
+            notice = "<div class='auto_fade notice' onclick='new Effect.Fade(this)'>#{::ERB::Util.html_escape(flash[:notice])}</div>"
           end
 
           if (type == 'error'  || type == 'both') && flash[:error ]
-            error = "<div class='error' onclick='new Effect.Fade(this)'>#{flash[:error]}</div>"
+            error = "<div class='error' onclick='new Effect.Fade(this)'>#{::ERB::Util.html_escape(flash[:error])}</div>"
           end
 
           if page = opts[:page]

diff --git a/lib/zena/use/i18n.rb b/lib/zena/use/i18n.rb
@@ -205,7 +205,7 @@ def set_encoding
       module ViewMethods
         include RubyLess
 
-        safe_method [:lang_links, {:wrap => String, :join => String}] => String
+        safe_method [:lang_links, {:wrap => String, :join => String}] => {:class => String, :method => 'lang_links', :html_safe => true}
 
         def self.included(base)
           base.send(:alias_method_chain, :will_paginate, :i18n) if base.respond_to?(:will_paginate)

diff --git a/lib/zena/use/query_builder.rb b/lib/zena/use/query_builder.rb
@@ -5,7 +5,7 @@ module QueryBuilder
       module ViewMethods
         include RubyLess
         safe_method [:query_parse, String] => {:class => String, :accept_nil => true}
-        safe_method :query_errors => {:class => String, :nil => true}
+        safe_method :query_errors => {:class => String, :nil => true, :html_safe => true}
 
         def find_node_by_zip(zip)
           return nil unless zip
@@ -34,7 +34,7 @@ def query(class_name, node_name, pseudo_sql, opts = {})
                 return klass.do_find(opts[:find] || :all, eval(query.to_s, opts[:binding] || binding))
               end
             rescue ::QueryBuilder::Error => err
-              @query_errors = "<span class='query'>#{pseudo_sql}</span> <span class='error'>#{err}</span>"
+              @query_errors = "<span class='query'>#{::ERB::Util.html_escape(pseudo_sql)}</span> <span class='error'>#{::ERB::Util.html_escape(err)}</span>"
             end
           end
           # error

diff --git a/lib/zena/use/rendering.rb b/lib/zena/use/rendering.rb
@@ -147,10 +147,11 @@ def render_and_cache(options={})
               }
             end
 
-            if result[:type] == 'text/html'
+            if error = result[:error]
               # error reporting from rendering engine
               opts[:cache] = false
-              render :text => result[:data]
+              @render_error = error
+              render :file => 'nodes/render_error'
             else
               if zafu_headers
                 if disposition = zafu_headers.delete('Content-Disposition')

diff --git a/lib/zena/use/zafu_safe_definitions.rb b/lib/zena/use/zafu_safe_definitions.rb
@@ -99,16 +99,16 @@ def self.join_proc
         safe_method :now    => {:method => 'Time.now', :class => Time}
         safe_method :string_hash => {:method => 'StringHash.new', :class => StringHash}
         safe_method [:string_hash, Hash] => {:method => 'StringHash.from_hash', :class => StringHash}
-        safe_method [:h, String] => {:class => String, :nil => true}
+        safe_method [:h, String] => {:class => String, :accept_nil => true}
         safe_method_for String, [:gsub, Regexp, String] => {:class => String, :pre_processor => true}
         safe_method_for String, :upcase    => {:class => String, :pre_processor => true}
         safe_method_for String, :strip     => {:class => String, :pre_processor => true}
         safe_method_for String, :urlencode => {:class => String, :pre_processor => true, :method => :urlencode}
         safe_method_for String, :url_name  => {:class => String, :pre_processor => true, :method => :url_name}
         safe_method_for String, :to_i      => {:class => Number, :pre_processor => true}
         safe_method_for String, :to_s      => {:class => String, :pre_processor => true}
-        safe_method_for String, [:limit, Number]  => {:class => String, :pre_processor => true}
-        safe_method_for String, [:limit, Number, String]  => {:class => String, :pre_processor => true}
+        safe_method_for String, [:limit, Number]  => {:class => String, :pre_processor => true, :html_safe => true}
+        safe_method_for String, [:limit, Number, String]  => {:class => String, :pre_processor => true, :html_safe => true}
         safe_method_for String, :to_f      => {:class => Number, :pre_processor => true}
         safe_method_for String, :to_json   => {:class => String, :pre_processor => true}
         safe_method_for String, [:split, String] => {:class => [String], :pre_processor => true}

diff --git a/test/integration/zafu_compiler/ajax.yml b/test/integration/zafu_compiler/ajax.yml
@@ -6,8 +6,8 @@ default:
 
 block:
   src: "<r:parent><r:block name='foobar' do='title'/></r:parent>"
-  tem: "<% if var1 = @node.parent %><div id='foobar' data-z='<%= var1.zip %>'><%= var1.prop['title'] %></div><% end %>"
-  'ajax/block/en/foobar.erb': "<div id='<%= ndom_id(@node) %>' data-z='<%= @node.zip %>'><%= @node.prop['title'] %></div>"
+  tem: "<% if var1 = @node.parent %><div id='foobar' data-z='<%= var1.zip %>'><%=h var1.prop['title'] %></div><% end %>"
+  'ajax/block/en/foobar.erb': "<div id='<%= ndom_id(@node) %>' data-z='<%= @node.zip %>'><%=h @node.prop['title'] %></div>"
 
 add:
   src: "<ul id='children' do='nodes'><li do='each' do='link'/><li do='add'/></ul>"
@@ -133,7 +133,7 @@ block_dictionary:
 make_form:
   src: "<ul do='children'><li do='each' do='title'/><li do='add'/></ul>"
   tem: "/<li style='display:none;' class='form' id='list1_0'>.*remote_form_for\(:node, var2_new"
-  'ajax/make/form/en/list1.erb': "<li id='<%= ndom_id(@node) %>'><%= @node.prop['title'] %></li>"
+  'ajax/make/form/en/list1.erb': "<li id='<%= ndom_id(@node) %>'><%=h @node.prop['title'] %></li>"
   'ajax/make/form/en/list1_form.erb': "/<li class='form' id='<%= ndom_id\(@node\) %>'>/"
 
 each_edit_cannot_write:
@@ -366,7 +366,7 @@ update_target_encode_params:
 
 include_update_target:
   src: "IUT: <r:include template='/ajax/update/target'><r:with part='foo'><r:show attr='title'/></r:with></r:include>"
-  tem: "/IUT: UT: <div .*id='foo'.*><%= h @node.prop\['title'\] %></div> <a .*zen_path.*onclick='new Ajax.Request/"
+  tem: "/IUT: UT: <div .*id='foo'.*><%=h @node.prop\['title'\] %></div> <a .*zen_path.*onclick='new Ajax.Request/"
 
 id_in_each_group_should_be_scoped:
   src: "<ul do='comments from nodes in site' do='group' by='discussion_id'><li do='each'><r:node do='block' do='title'/></li></ul>"
@@ -422,13 +422,13 @@ js:
 
 js_dyn:
   src: "x <r:js>alert('ho <r:title/>');</r:js> y"
-  tem: "x <% js_data << capture do %>alert('ho <%= @node.prop['title'] %>');<% end %> y"
+  tem: "x <% js_data << capture do %>alert('ho <%=h @node.prop['title'] %>');<% end %> y"
   res: "x  y"
   js: "<script type=\"text/javascript\">\n//<![CDATA[\nalert('ho status title');\n//]]>\n</script>"
 
 js_less_then:
   src: "x <r:js>if (i < 4) alert('ho <r:title/>');</r:js> y"
-  tem: "x <% js_data << capture do %>if (i < 4) alert('ho <%= @node.prop['title'] %>');<% end %> y"
+  tem: "x <% js_data << capture do %>if (i < 4) alert('ho <%=h @node.prop['title'] %>');<% end %> y"
   res: "x  y"
   js: "<script type=\"text/javascript\">\n//<![CDATA[\nif (i < 4) alert('ho status title');\n//]]>\n</script>"
 

diff --git a/test/integration/zafu_compiler/comments.yml b/test/integration/zafu_compiler/comments.yml
@@ -14,7 +14,7 @@ comments_shown_if_empty_but_can_comment:
 
 discussion:
   src: "<r:discussion do='comments' do='title'></r:discussion>"
-  tem: "<% if var1 = @node.discussion %><% if var2 = var1.comments %><%= var2.first.title %><% end %><% end %>"
+  tem: "<% if var1 = @node.discussion %><% if var2 = var1.comments %><%=h var2.first.title %><% end %><% end %>"
   # no error
   res: "What about rivers ?"
 

diff --git a/test/integration/zafu_compiler/display.yml b/test/integration/zafu_compiler/display.yml
@@ -49,20 +49,17 @@ multiline_tag:
       </div>
     </div>
   tem: "!/do='pages'/"
-
 
 show_title:
-  old_src: "<r:show attr='title'/>"
-  old_tem: "<%= @node.title %>"
   src: "<r:title/>"
-  tem: "<%= @node.prop['title'] %>"
+  tem: "<%=h @node.prop['title'] %>"
   res: "status title"
 
 show_title_with_opts:
   old_src: "<h1 do='title' class='s70' status='true' actions='all'>this is the title</h1>"
   old_tem: "/<h1 class='s<%= @node.version.status %>'><%= show_title\(:node=>@node\) \+ node_actions\(:node=>@node, .*:actions=>\"all\"/"
   src: "<h1 do='title' prefix='status' actions='all' live='true'>this is the title</h1>"
-  tem: "<h1 class='s<%= @node.version.status %>'><span id='_title<%= @node.zip %>'><%= h @node.prop['title'] %></span> <%= node_actions(@node, :actions => \"all\") %></h1>"
+  tem: "<h1 class='s<%= @node.version.status %>'><span id='_title<%= @node.zip %>'><%=h @node.prop['title'] %></span> <%= node_actions(@node, :actions => \"all\") %></h1>"
   old_res: "/<h1 class='s50'><span id='title22'.*class='actions'>/"
   res: "/<h1 class='s50'><span id='_title22'.*class='actions'>/"
 
@@ -90,7 +87,7 @@ show_title_in_list:
 
 do_title:
   src: "<h2 do='title'/>"
-  tem: "<h2><%= @node.prop['title'] %></h2>"
+  tem: "<h2><%=h @node.prop['title'] %></h2>"
   res: "<h2>status title</h2>"
 
 show_title_link_id_from_stored:
@@ -140,7 +137,7 @@ title_in_version_context:
 show_shortcut:
   old_src: "<p do='[title]'>hello</p>"
   src: "<p do='title'>hello</p>"
-  tem: "<p><%= @node.prop['title'] %></p>"
+  tem: "<p><%=h @node.prop['title'] %></p>"
   res: "<p>status title</p>"
 
 zazen_shortcut:
@@ -184,13 +181,13 @@ show_width:
 show_else:
   old_src: "<r:show attr='comment' else='name'/>"
   src: "<r:show eval='origin || title'/>"
-  tem: "<%= h (@node.prop['origin'] or @node.prop['title']) %>"
+  tem: "<%=h (@node.prop['origin'] or @node.prop['title']) %>"
   res: "status title"
 
 show_default:
   old_src: "<r:show attr='d_foo' default='baz'/>"
   src: "<r:show eval='origin || \"baz\"'/>"
-  tem: "<%= h (@node.prop['origin'] or \"baz\") %>"
+  tem: "<%=h (@node.prop['origin'] or \"baz\") %>"
   res: "baz"
 
 javascripts:
@@ -330,7 +327,7 @@ show_with_label_shortcut:
   context:
     lang: fr
   src: "<li do='show' label='t' blank='hide' attr='title'/>"
-  tem: "<% if !@node.prop['title'].blank? %><li><label>titre</label> <span><%= h @node.prop['title'] %></span></li><% end %>"
+  tem: "<% if !@node.prop['title'].blank? %><li><label>titre</label> <span><%=h @node.prop['title'] %></span></li><% end %>"
   res: "<li><label>titre</label> <span>Etat des travaux</span></li>"
 
 show_with_custom_label_shortcut:
@@ -414,15 +411,15 @@ short_path:
 show_param:
   context:
     t: 'hello'
-  src: "<r:show param='t'/>"
-  tem: "<%= h params[:t] %>"
+#  src: "<r:show param='t'/><r:show param='x'/>"
+  tem: "<%=h params[:t] %><%=h params[:x] %>"
   res: "hello"
 
 display_hash_type:
   context:
     node: test
   src: "<r:void do='settings[\"one\"]'/>"
-  tem: "<%= (@node.prop['settings'] ? @node.prop['settings'][\"one\"] : nil) %>"
+  tem: "<%=h (@node.prop['settings'] ? @node.prop['settings'][\"one\"] : nil) %>"
   res: "un"
 
 default_host: