Bugs in REXML

Hi,

I think the following behaviors of REXML can be considered
as bugs but I
wanted a small confirmation before implementing some
patches:

1) XML Documents accept more than 1 XMLDecl (which is not
allowed by
http://www.w3.org
/TR/xml/#NT-prolog)

irb(main):001:0> require 'rexml/document'
irb(main):002:0> d = REXML:ocument.
new("<foo />")
irb(main):004:0> d.add REXML::XMLDecl.new
irb(main):005:0> d.add REXML::XMLDecl.new
=> [<?xml ... ?>, <?xml ... ?>,
<foo/>]
irb(main):006:0> d.write
<?xml version='1.0'?><?xml
version='1.0'?><foo/>

This behavior can be seen with "Document#to_s" and
"Document#children"
too. It goes back to:

ruby_1_9/lib/rexml/document.rb:70
  if child.kind_of? XMLDecl
        children.unshift child
        child.parent = self
  elsif...

Since it's not verifying if a XMLDecl already exists for the
document.

2) DocTypes are being added to the end of the document.

This bug has been reported in
http://www.germane-software.com/projects/rexml/ticket/1
20 for at least 4
months.
To add a bit more, I _think_ this condition will fail
(kind_of without "?"
unless it's defined like that somewhere else) but it'll
never be met:

ruby_1_9/lib/rexml/document.rb:80
  if children[ insert_before_index ] # Not null = not end
of list
    if children[ insert_before_index ].kind_of DocType

Since insert_before_index will be the expected position +
1.

Again, as I said earlier, I'm not entirely sure if my
reasonings are fine
but I know the errors are there in 1.8 and probably 1.9
(unless I'm
missing something).

Comments?

-- 
Federico

On 3/21/08, Federico Builes <federico.builesgmail.com> wrote:
>  To add a bit more, I _think_ this condition will fail
(kind_of without "?"
>  unless it's defined like that somewhere else) but
it'll never be met:

To correct myself, it'll indeed fail with this piece of
code:

    d = REXML:ocument.
new("<root/>")
    d.add REXML:ocType.n
ew("foo")
    d.add REXML:ocType.n
ew("bar")

According to the doc:

/ruby_1_9/lib/rexml/document.rb:73
        # ...  If there is a child and it is an DocType,
then replace it.

-- 
Federico

On Mar 21, 2008, at 17:35, Federico Builes wrote:
> I think the following behaviors of REXML can be
considered as bugs  
> but I
> wanted a small confirmation before implementing some
patches:
>
> 1) XML Documents accept more than 1 XMLDecl (which is
not allowed by
> http://www.w3.org
/TR/xml/#NT-prolog)

As the REXML web page says,

> One thing to be aware of is that REXML is not (yet) a
validating  
> parser. This means that some invalid namespace
declarations are not  
> caught.


Seems to me that accepting more than 1 XML declaration is
entirely  
acceptable, for a parser which states that it is
non-validating.


mathew

On 3/22/08, mathew <metapobox.com> wrote:
>  Seems to me that accepting more than 1 XML declaration
is entirely
>  acceptable, for a parser which states that it is
non-validating.

Fair enough, no fix for this then.

About the second issue: A doctype at the end of the document
would be
invalid too, but the purpose of the current source code is
to add it
between the XMLDecl and the first node, replacing an
existing doctype
if there's already one. Can this be fixed or should it stay
that way
too?

-- 
Federico

At 05:30 08/03/23, mathew wrote:

>Seems to me that accepting more than 1 XML declaration
is entirely  
>acceptable, for a parser which states that it is
non-validating.

No, sorry, not at all. The XML declaration is part of
well-formedness,
not validity. So is the number and place of document type
declarations
or similar stuff. If REXML gets this wrong, it should be
fixed asap.

As a separate issue, it seems that REXML doesn't support the
namespace
axis in XPath. I hope this can be fixed, too.

Regards,    Martin.


#-#-#  Martin J. Du"rst, Assoc. Professor, Aoyama
Gakuin University
#-#-#  http://www.sw.it.aoyama
.ac.jp       mailto:duerstit.aoyama.ac.jp     

On 3/23/08, Martin Duerst <duerstit.aoyama.ac.jp> wrote:
> No, sorry, not at all. The XML declaration is part of
well-formedness,
>  not validity. So is the number and place of document
type declarations
>  or similar stuff. If REXML gets this wrong, it should
be fixed asap.

I've attached a bug fix for this in the tracker:
http://rubyforge.
org/tracker/index.php?func=detail&aid=19058&group_id
=426&atid=1698

Also, while writing REXML's spec for Rubinius I've found
some other
minor problems, I've attached fixes for them too:

* Fix for get_attribute in REXML::Attribute -
http://rubyforge.
org/tracker/index.php?func=detail&aid=19039&group_id
=426&atid=1698

* Fix for standalone's default value in XMLDecl -
http://rubyforge.
org/tracker/index.php?func=detail&aid=19032&group_id
=426&atid=1698

* Docfix for Document -
http://rubyforge.
org/tracker/index.php?func=detail&aid=19038&group_id
=426&atid=1698


-- 
Federico

Federico Builes wrote:
> On 3/23/08, Martin Duerst <duerstit.aoyama.ac.jp> wrote:
>> No, sorry, not at all. The XML declaration is part
of well-formedness,
>>  not validity. So is the number and place of
document type declarations
>>  or similar stuff. If REXML gets this wrong, it
should be fixed asap.
> 
> I've attached a bug fix for this in the tracker:
> http://rubyforge.
org/tracker/index.php?func=detail&aid=19058&group_id
=426&atid=1698
> 
> Also, while writing REXML's spec for Rubinius I've
found some other
> minor problems, I've attached fixes for them too:
> 
> * Fix for get_attribute in REXML::Attribute -
> http://rubyforge.
org/tracker/index.php?func=detail&aid=19039&group_id
=426&atid=1698
> 
> * Fix for standalone's default value in XMLDecl -
> http://rubyforge.
org/tracker/index.php?func=detail&aid=19032&group_id
=426&atid=1698
> 
> * Docfix for Document -
> http://rubyforge.
org/tracker/index.php?func=detail&aid=19038&group_id
=426&atid=1698

Please don't take this as being ungrateful, but in order to
prevent 
future regressions, any chance that test cases for the
issues that these 
fixes address can be added?

- Sam Ruby

On 3/23/08, Sam Ruby <rubysintertwingly.net>
wrote:
> Federico Builes wrote:
>  > * Fix for standalone's default value in XMLDecl
-
>  > http://rubyforge.
org/tracker/index.php?func=detail&aid=19032&group_id
=426&atid=1698

I've closed my patch as rejected since the standalone
value's not
mandatory. The patch made it appear on every single XMLDecl
if nothing
was specified so I preferred to just remove it.

I still think that docs for Document#stand_alone? should be
updated to
reflect that if no standalone value has been given, nil will
be
returned (not the default value as the docs kindly say).

> Please don't take this as being ungrateful, but in
order to prevent
>  future regressions, any chance that test cases for the
issues that these
>  fixes address can be added?

I'm attaching the tests.
It should've been done in the last email but since I didn't
see a
folder for rexml in tests/ I skipped that step, that was my
bad.

This patch creates a simple test_rexml file covering the
issues
pointed in the mail and is by no means exhaustive. Please
take a look
and if you see something's missing please let me know.

-- 
Federico