generalize and test sanitization of unescaped ampersands in OAI responses

Author: barmintor

lib/oai/client.rb

@@ -54,7 +54,7 @@ module OAI
   # <http://www.openarchives.org/OAI/openarchivesprotocol.html>.
 
   class Client
-
+    UNESCAPED_AMPERSAND = /&(?!(?:amp|lt|gt|quot|apos|\#\d+);)/
     # The constructor which must be passed a valid base url for an oai
     # service:
     #
@@ -198,20 +198,25 @@ def list_sets(opts={})
       do_resumable(OAI::ListSetsResponse, 'ListSets', opts)
     end
 
-    private
-
-    def do_request(verb, opts = nil)
-      # fire off the request and return appropriate DOM object
-      uri = build_uri(verb, opts)
-      xml = strip_invalid_utf_8_chars(get(uri))
+    def sanitize_xml(xml)
+      xml = strip_invalid_utf_8_chars(xml)
+      xml = strip_invalid_xml_chars(xml)
       if @parser == 'libxml'
         # remove default namespace for oai-pmh since libxml
         # isn't able to use our xpaths to get at them
         # if you know a way around thins please let me know
         xml = xml.gsub(
           /xmlns=\"http:\/\/www.openarchives.org\/OAI\/.\..\/\"/, '')
       end
-      return load_document(xml)
+      xml
+    end
+
+    private
+
+    def do_request(verb, opts = nil)
+      # fire off the request and return appropriate DOM object
+      uri = build_uri(verb, opts)
+      return load_document(get(uri))
     end
 
     def do_resumable(responseClass, verb, opts)
@@ -241,6 +246,7 @@ def encode(value)
     end
 
     def load_document(xml)
+      xml = sanitize_xml(xml)
       case @parser
       when 'libxml'
         begin
@@ -251,7 +257,6 @@ def load_document(xml)
         end
       when 'rexml'
         begin
-          xml = strip_invalid_xml_chars(xml)
           return REXML::Document.new(xml)
         rescue REXML::ParseException => e
           raise OAI::Exception, 'response not well formed XML: '+e.message, caller
@@ -356,17 +361,8 @@ def strip_invalid_utf_8_chars(xml)
     end
 
     def strip_invalid_xml_chars(xml)
-      invalid = false
-
-      begin
-        REXML::Document.new(xml)
-      rescue REXML::ParseException => e
-        invalid = true
-      end
-
-      return xml.gsub!(/&(?!(?:amp|lt|gt|quot|apos);)/, '&amp;') if invalid
-      return xml
+      return xml unless xml =~ UNESCAPED_AMPERSAND
+      xml.gsub(UNESCAPED_AMPERSAND, '&amp;')
     end
-
   end
 end

test/client/tc_utf8_escaping.rb

@@ -1,12 +1,19 @@
 require 'test_helper_client'
 
 class UTF8Test < Test::Unit::TestCase
+  def client
+    @client ||= OAI::Client.new 'http://localhost:3333/oai'
+  end
 
   def test_escaping_invalid_utf_8_characters
-    client = OAI::Client.new 'http://localhost:3333/oai' #, :parser => 'libxml'
     invalid_utf_8 = [2, 3, 4, 104, 5, 101, 6, 108, 66897, 108, 66535, 111, 1114112, 33, 55234123, 33].pack("U*")
     invalid_utf_8 = invalid_utf_8.force_encoding("binary") if invalid_utf_8.respond_to? :force_encoding
     assert_equal("hello!!", client.send(:strip_invalid_utf_8_chars, invalid_utf_8).gsub(/\?/, ''))
   end
 
+  def test_unescaped_ampersand_content_correction
+    src = '<test>Frankie & Johnny <character>&#9829;</character></test>'
+    expected = '<test>Frankie &amp; Johnny <character>&#9829;</character></test>'
+    assert_equal(expected, client.sanitize_xml(src))
+  end
 end