You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by tc...@redhat.com on 2010/11/24 17:45:40 UTC

[PATCH] Wrap error messages and details in CDATA

From: Tobias Crawley <tc...@redhat.com>

The xml parser used by nokogiri-java (xerces) is much more strict about
unescaped entities, and causes nokogiri to ignore nodes containing them.
Wrapping the content in CDATA blocks fixes this.

Note: this fix does not use the haml :cdata filter, since you cannot
strip whitespace inside the filter.
---
 .../lib/deltacloud/helpers/application_helper.rb   |    4 ++--
 server/views/errors/auth_exception.xml.haml        |    3 ++-
 .../errors/backend_capability_failure.xml.haml     |    3 ++-
 server/views/errors/backend_error.xml.haml         |    4 ++--
 server/views/errors/validation_failure.xml.haml    |    5 +++--
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/server/lib/deltacloud/helpers/application_helper.rb b/server/lib/deltacloud/helpers/application_helper.rb
index 00e8bc9..6830e5f 100644
--- a/server/lib/deltacloud/helpers/application_helper.rb
+++ b/server/lib/deltacloud/helpers/application_helper.rb
@@ -121,8 +121,8 @@ module ApplicationHelper
     end
   end
 
-  def cdata(&block)
-    text = capture_haml(&block)
+  def cdata(text = nil, &block)
+    text ||= capture_haml(&block)
     "<![CDATA[#{text.strip}]]>"
   end
 
diff --git a/server/views/errors/auth_exception.xml.haml b/server/views/errors/auth_exception.xml.haml
index bee6492..bfa9111 100644
--- a/server/views/errors/auth_exception.xml.haml
+++ b/server/views/errors/auth_exception.xml.haml
@@ -1,2 +1,3 @@
 %error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %message #{@error.message}
+  %message< #{cdata @error.message}
+
diff --git a/server/views/errors/backend_capability_failure.xml.haml b/server/views/errors/backend_capability_failure.xml.haml
index 83892fb..4302e4b 100644
--- a/server/views/errors/backend_capability_failure.xml.haml
+++ b/server/views/errors/backend_capability_failure.xml.haml
@@ -1,4 +1,5 @@
 %error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
   %capability #{@error.capability}
-  %message #{@error.message}
+  %message< #{cdata @error.message}
+
 
diff --git a/server/views/errors/backend_error.xml.haml b/server/views/errors/backend_error.xml.haml
index 75866eb..cb5d87f 100644
--- a/server/views/errors/backend_error.xml.haml
+++ b/server/views/errors/backend_error.xml.haml
@@ -4,5 +4,5 @@
     %code= @error.code
     %cause= @error.cause
     - if @error.details
-      %details #{@error.details}
-  %message #{@error.message}
+      %details< #{cdata @error.details.join("\n")}
+  %message< #{cdata @error.message}
diff --git a/server/views/errors/validation_failure.xml.haml b/server/views/errors/validation_failure.xml.haml
index 24519ed..f18d6a2 100644
--- a/server/views/errors/validation_failure.xml.haml
+++ b/server/views/errors/validation_failure.xml.haml
@@ -1,7 +1,8 @@
 %error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
   %parameter #{@error.name}
-  %message #{@error.message}
+  %message< #{cdata @error.message}
   - unless @error.param.options.empty?
     %valid_options
       - @error.param.options.each do |v|
-        %value #{v}
+        %value< #{cdata v}
+
-- 
1.7.3.2


Re: [PATCH] Wrap error messages and details in CDATA

Posted by Michal Fojtik <mf...@redhat.com>.
On 24/11/10 11:45 -0500, tcrawley@redhat.com wrote:
>From: Tobias Crawley <tc...@redhat.com>
>
>The xml parser used by nokogiri-java (xerces) is much more strict about
>unescaped entities, and causes nokogiri to ignore nodes containing them.
>Wrapping the content in CDATA blocks fixes this.
>

ACK. Code looks safe and I understand your motivation, XML can get corrupted
when Exception contains invalid characters.

   -- Michal

>
>Note: this fix does not use the haml :cdata filter, since you cannot
>strip whitespace inside the filter.
>---
> .../lib/deltacloud/helpers/application_helper.rb   |    4 ++--
> server/views/errors/auth_exception.xml.haml        |    3 ++-
> .../errors/backend_capability_failure.xml.haml     |    3 ++-
> server/views/errors/backend_error.xml.haml         |    4 ++--
> server/views/errors/validation_failure.xml.haml    |    5 +++--
> 5 files changed, 11 insertions(+), 8 deletions(-)
>
>diff --git a/server/lib/deltacloud/helpers/application_helper.rb b/server/lib/deltacloud/helpers/application_helper.rb
>index 00e8bc9..6830e5f 100644
>--- a/server/lib/deltacloud/helpers/application_helper.rb
>+++ b/server/lib/deltacloud/helpers/application_helper.rb
>@@ -121,8 +121,8 @@ module ApplicationHelper
>     end
>   end
>
>-  def cdata(&block)
>-    text = capture_haml(&block)
>+  def cdata(text = nil, &block)
>+    text ||= capture_haml(&block)
>     "<![CDATA[#{text.strip}]]>"
>   end
>
>diff --git a/server/views/errors/auth_exception.xml.haml b/server/views/errors/auth_exception.xml.haml
>index bee6492..bfa9111 100644
>--- a/server/views/errors/auth_exception.xml.haml
>+++ b/server/views/errors/auth_exception.xml.haml
>@@ -1,2 +1,3 @@
> %error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
>-  %message #{@error.message}
>+  %message< #{cdata @error.message}
>+
>diff --git a/server/views/errors/backend_capability_failure.xml.haml b/server/views/errors/backend_capability_failure.xml.haml
>index 83892fb..4302e4b 100644
>--- a/server/views/errors/backend_capability_failure.xml.haml
>+++ b/server/views/errors/backend_capability_failure.xml.haml
>@@ -1,4 +1,5 @@
> %error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
>   %capability #{@error.capability}
>-  %message #{@error.message}
>+  %message< #{cdata @error.message}
>+
>
>diff --git a/server/views/errors/backend_error.xml.haml b/server/views/errors/backend_error.xml.haml
>index 75866eb..cb5d87f 100644
>--- a/server/views/errors/backend_error.xml.haml
>+++ b/server/views/errors/backend_error.xml.haml
>@@ -4,5 +4,5 @@
>     %code= @error.code
>     %cause= @error.cause
>     - if @error.details
>-      %details #{@error.details}
>-  %message #{@error.message}
>+      %details< #{cdata @error.details.join("\n")}
>+  %message< #{cdata @error.message}
>diff --git a/server/views/errors/validation_failure.xml.haml b/server/views/errors/validation_failure.xml.haml
>index 24519ed..f18d6a2 100644
>--- a/server/views/errors/validation_failure.xml.haml
>+++ b/server/views/errors/validation_failure.xml.haml
>@@ -1,7 +1,8 @@
> %error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
>   %parameter #{@error.name}
>-  %message #{@error.message}
>+  %message< #{cdata @error.message}
>   - unless @error.param.options.empty?
>     %valid_options
>       - @error.param.options.each do |v|
>-        %value #{v}
>+        %value< #{cdata v}
>+
>--
>1.7.3.2
>

-- 
--------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
--------------------------------------------------------