On Wed, Sep 08, 2010 at 07:00:20PM +0200, Mike Hommey wrote:
> Hi,
> 
> Using the SVG xsd schema, and even just parsing it with xmllint fails:
> 
> $ xmllint http://www.w3.org/TR/2002/WD-SVG11-20020108/SVG.xsd
> http://www.w3.org/TR/2002/WD-SVG11-20020108/SVG.xsd:12: error: 
> xmlSAX2StartElementNs: out of memory
> lementFormDefault="unqualified" attributeFormDefault="unqualified" 
> xml:lang="en"
>                                                                               
>  ^
> http://www.w3.org/TR/2002/WD-SVG11-20020108/SVG.xsd:14: parser error : Extra 
> content at the end of the document
>   <import namespace="http://www.w3.org/1999/xlink"; 
> schemaLocation="xlink.xsd"/>
> 
> The error message is actually absolutely wrong, since there is
> not much memory used (valgrind says around 80KB).

  yup, I think the code initially expected that to return NULL only
in case of allocation error (if the node is not provided)  but the
function was changed because adding those xml namespace declarations
is not needed.

> What happens is that xmlNewNs() fails in xmlSAX2StartElementNs because
> what is passed to xmlNewNs fits the following:
> 
> if ((prefix != NULL) && (xmlStrEqual(prefix, BAD_CAST "xml")))
>     return(NULL);

 it's a bit more complex than that, acually one can declare the
xml prefix, assuming it uses the right namespace name, but it's
basically useless, because it's automatically assumed by parsers.

> xmlSAX2StartElementNs throws an OOM error when xmlNewNs returns NULL...

  that is wrong, this is the core bug, it should ignore it considering
the new behaviour of the function.

> I don't know if it's supposed to be valid to use
> xmlns:xml="http://www.w3.org/XML/1998/namespace"; in a xml file, though.

 yes, just unecessary :-)

> >From a quick glance at the spec, while it says
> "The prefix xml is by definition bound to the namespace name
> http://www.w3.org/XML/1998/namespace.";, it doesn't seem to say anything
> about explicitely writing it being forbidden...

  attached patch fixes the issue for me, I will commit it soonish

> PS: Posting here, because it seems bugzilla is not read... I have 3 patches
> in there that are bitrotting. (613466, 613467, 625851)

  argh, well yes for urgent issues it's good to also raise problems here
but bugzilla is important too to make sure the issue is kept. I usually
go though bugzilla when I think I have time for a release, which doesn't
happen very often.

quickly:
  613466 ACK, makes sense patch looks just fine
  613467 ACK, I tend to not build out of tree, looks fine
  625851 nontrivial, patch seems fine, but the swap of thread and global
         init I need to double check to make sure the thread local storage
         is still properly allocated in that case

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[email protected]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/
diff --git a/SAX2.c b/SAX2.c
index 84c1f00..c0482c0 100644
--- a/SAX2.c
+++ b/SAX2.c
@@ -2242,8 +2242,12 @@ xmlSAX2StartElementNs(void *ctx,
            if ((URI != NULL) && (prefix == pref))
                ret->ns = ns;
        } else {
-           xmlSAX2ErrMemory(ctxt, "xmlSAX2StartElementNs");
-           return;
+            /*
+             * any out of memory error would already have been raised
+             * but we can't be garanteed it's the actual error due to the
+             * API, best is to skip in this case
+             */
+           continue;
        }
 #ifdef LIBXML_VALID_ENABLED
        if ((!ctxt->html) && ctxt->validate && ctxt->wellFormed &&
diff --git a/tree.c b/tree.c
index 1e1a23a..24db82a 100644
--- a/tree.c
+++ b/tree.c
@@ -721,8 +721,19 @@ xmlNewNs(xmlNodePtr node, const xmlChar *href, const 
xmlChar *prefix) {
     if ((node != NULL) && (node->type != XML_ELEMENT_NODE))
        return(NULL);
 
-    if ((prefix != NULL) && (xmlStrEqual(prefix, BAD_CAST "xml")))
-       return(NULL);
+    if ((prefix != NULL) && (xmlStrEqual(prefix, BAD_CAST "xml"))) {
+        /* xml namespace is predefined, no need to add it */
+        if (xmlStrEqual(href, XML_XML_NAMESPACE))
+            return(NULL);
+
+        /*
+         * Problem, this is an attempt to bind xml prefix to a wrong
+         * namespace, which breaks
+         * Namespace constraint: Reserved Prefixes and Namespace Names
+         * from XML namespace. But documents authors may not care in
+         * their context so let's proceed.
+         */
+    }
 
     /*
      * Allocate a new Namespace and fill the fields.
_______________________________________________
xml mailing list, project page  http://xmlsoft.org/
[email protected]
http://mail.gnome.org/mailman/listinfo/xml

Reply via email to