From 61b02c59ecb917692bd231769e314ef42d699142 Mon Sep 17 00:00:00 2001
From: TSUYUSATO Kitsune <make.just.on@gmail.com>
Date: Tue, 18 Jun 2024 18:22:12 +0900
Subject: [PATCH] Handle unprefixed attributes in `Element#attribute` correctly

Currently, `Element#attrubute` handles unprefixed attributes as
attributes with the default namespace, and handles the default
namespace as `nil` value. However, it is a mis-interpretation of
the specification of XML 1.0.

XML 1.0 specification says:
(https://www.w3.org/TR/xml-names/#defaulting)

> The namespace name for an unprefixed attribute name always has
> no value.

Therefore, we need to distinguish `nil` and the default namespace.
Also, unprefixed attributes should not be handled as attributes
with the default namespace.

Note that XML 1.0 specification also says:

> Default namespace declarations do not apply directly to attribute
> names; the interpretation of unprefixed attributes is determined
> by the element on which they appear.

This means that we can know (programmatically) how to handle
unprefixed attributes from the element's namespace. It does not mean that
the namespace of an unprefixed attribute becomes the default namespace.

This commit changes `Element#attribute` behavior correctly and updates
tests for it.
---
 lib/rexml/element.rb | 19 +++----------------
 test/test_core.rb    | 32 ++++++++++++++------------------
 2 files changed, 17 insertions(+), 34 deletions(-)

diff --git a/lib/rexml/element.rb b/lib/rexml/element.rb
index a5808d7c..540261f8 100644
--- a/lib/rexml/element.rb
+++ b/lib/rexml/element.rb
@@ -1270,27 +1270,14 @@ def [](name_or_index)
     # With arguments +name+ and +namespace+ given,
     # returns the value of the named attribute if it exists, otherwise +nil+:
     #
-    #   xml_string = "<root xmlns:a='a' a:x='a:x' x='x'/>"
+    #   xml_string = "<root xmlns:a='http://example.com/a' a:x='a:x' x='x'/>"
     #   document = REXML::Document.new(xml_string)
     #   document.root.attribute("x")      # => x='x'
-    #   document.root.attribute("x", "a") # => a:x='a:x'
+    #   document.root.attribute("x", "http://example.com/a") # => a:x='a:x'
     #
     def attribute( name, namespace=nil )
       prefix = namespaces.key(namespace) if namespace
-      prefix = nil if prefix == 'xmlns'
-
-      ret_val =
-        attributes.get_attribute( prefix ? "#{prefix}:#{name}" : name )
-
-      return ret_val unless ret_val.nil?
-      return nil if prefix.nil?
-
-      # now check that prefix'es namespace is not the same as the
-      # default namespace
-      return nil unless ( namespaces[ prefix ] == namespaces[ 'xmlns' ] )
-
-      attributes.get_attribute( name )
-
+      attributes.get_attribute( prefix ? "#{prefix}:#{name}" : name )
     end
 
     # :call-seq:
diff --git a/test/test_core.rb b/test/test_core.rb
index 44e2e7ea..2566edbb 100644
--- a/test/test_core.rb
+++ b/test/test_core.rb
@@ -1451,12 +1451,6 @@ def test_ticket_95
       assert_equal(out1,out2)
     end
 
-    def test_ticket_102
-      doc = REXML::Document.new '<doc xmlns="ns"><item name="foo"/></doc>'
-      assert_equal( "foo", doc.root.elements["*:item"].attribute("name","ns").to_s )
-      assert_equal( "item", doc.root.elements["*:item[@name='foo']"].name )
-    end
-
     def test_ticket_14
       # Per .2.5 Node Tests of XPath spec
       assert_raise( REXML::UndefinedNamespaceException,
@@ -1478,18 +1472,6 @@ def test_ticket_105
       assert_equal( 1, d.root.children.size )
     end
 
-    # phantom namespace same as default namespace
-    def test_ticket_121
-      doc = REXML::Document.new(
-        '<doc xmlns="ns" xmlns:phantom="ns"><item name="foo">text</item></doc>'
-      )
-      assert_equal 'text', doc.text( "/*:doc/*:item[@name='foo']" )
-      assert_equal "name='foo'",
-        doc.root.elements["*:item"].attribute("name", "ns").inspect
-      assert_equal "<item name='foo'>text</item>",
-        doc.root.elements["*:item[@name='foo']"].to_s
-    end
-
     def test_ticket_135
       bean_element = REXML::Element.new("bean")
       textToAdd = "(&#38;(|(memberof=CN=somegroupabcdefgh,OU=OUsucks,DC=hookemhorns,DC=com)(mail=*someco.com))(acct=%u)(!(extraparameter:2.2.222.222222.2.2.222:=2)))"
@@ -1521,6 +1503,20 @@ def test_ticket_138
                    REXML::Document.new(doc.root.to_s).root.attributes.to_h)
     end
 
+    def test_unprefixed_attribute
+      doc = REXML::Document.new(<<~XML)
+        <root xmlns="http://example.org/test">
+          <foo bar="baz" />
+        </root>
+      XML
+
+      assert_equal("baz", doc.elements["//foo"].attribute("bar")&.value)
+
+      # Unprefixed attributes have no prefix and the default namespace is not applied.
+      # See https://www.w3.org/TR/xml-names/#defaulting.
+      assert_equal(nil, doc.elements["//foo"].attribute("bar", "http://example.org/test")&.value)
+    end
+
     def test_empty_doc
       assert(REXML::Document.new('').children.empty?)
     end