Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:104692
To: Pierre Joye <pierre.php@gmail.com>, Benjamin Eberlei <kontakt@beberlei.de>
Cc: PHP internals <internals@lists.php.net>
References: <CAEPJdu=BuS8rf7_=GikUtCArdHSUD4AqY3+27tAJ1HPCP4Jm4A@mail.gmail.com>
 <CAEZPtU5r9oFDdDj_81+u+a2ZUQk3EjQ8FK1TvOnsODHGJUUVqw@mail.gmail.com>
Openpgp: preference=signencrypt
Autocrypt: addr=rrichards@ctindustries.net; prefer-encrypt=mutual; keydata=
 mQINBFo8IZ4BEADQXfpzxwmnlHpx333ClqJYJ3A6fZN+Es0QLUH3iE6l3E+Od84s4r3g/BxT
 4owOdkZoT6FtFH2bRpdtzDtscR+CNicgnQ1y0Ke3eW7hUB2zO/Nj27mrHSd5m8vaWniAQgwr
 /gW1ARArIuTFOTykc2/ttP0CcmhLGKa/js48oa/lN9RKUhWWeySRUADaaPth21rPtwUKCfe1
 /SnOCdfhX/qDh0HGetay3P5dd3rvtxKCXxbIUmF6onL8p0JqEop8/URHUD+FUCYlzUkrmZMS
 EEpdAN8tFvQzVJUg4tCU+c68WuZHPzWfFgiv0ZAKJOa5yLpyTCySQ09onyaPhzauCi31zcTQ
 jTjS8z7wVDEjXVR9xGs8b3wz5gDkXWXVVyywoRw9A8ZR92SHNeQee0NakCLFPnRj+fBT26LV
 FMYgw3G4f23CJDAA4lw4sURtnn/USpBYPB1oj1W8isnsqwefLdzgUKEIJsHtj3+8aaD6AXCY
 YwDHkBP9AopQgCO0KvImiLLiGurGOETcM9oUGuPiv7BMW6Sz5Hh2i8MrWn8m65Uo95OTl4f4
 4hHfbjP0ce8+pvWUlpfzo+RZBhb1FktbqWukLzqeDXJQInVuIi/bxi/dpfV20zy54hPK1fNV
 3o8woYzjWKoVUbMCT8FJzM524x9x1XBOuADZyLihZt3P3hHlQQARAQABtCZSb2IgUmljaGFy
 ZHMgPHJyaWNoYXJkc0BjZGF0YXpvbmUub3JnPokCVAQTAQgAPhYhBMDtUeC73vfKhx5FJX5Q
 8UtPNQs3BQJaPCGeAhsDBQkHhh+ABQsJCAcCBhUICQoLAgQWAgMBAh4BAheAAAoJEH5Q8UtP
 NQs3oYkP/1LzIMd/TBqNh31xtRW87UXFi/dQMHtGFRyr+aKHf0sv4vxUQsVZokU1ZD0gWRE0
 hvr68VPAOU/zaiv6QIQYxwdD3xk3ma7WdRxsGqDvka0EDCWMKUcPppQ5raSydWOWlp2n+AUl
 b0zulcPb6eLVg1indEDvPunfUYfM/zg4xtS3S38QqavnM2a295bteVG7J7cWISFDEGauRnq4
 kvoXuLXIjY9Ceji1MRUsFScz405/ut3PiFy++OrMdR4OGQuxQgEb8YmB/MXiS25pXFVnsr3u
 IaMRpILBnw8c6R69ejM8my5w2WXxRXrKaaa2j1V/tNYJ875GBGg0W5jChXPV1s1Uzj+pfy6t
 JpVhsnb5GmZ8Q8UxS8KYogdFACFF6ha20M7Bsh9GqXsRM7pl1H++yQZsmrkkC+ZD9sOuLGzk
 E87D7t8XFd07fkguyf4zkkmIeJY3VSlpYEHZiW3Tj0sBwtuGxZ7XtVXafyqXFRFDssR0C6A6
 qxdI6MVdWvDORTWqjOxe3HXrAm/rXqvyjmV21jON9VlZtG6seDh8p6A6JML8xdiuL/DFfDhS
 hf/2ulwY3qvEYZq5Yp93eg2IjjBZZ2PoG03+uUQXcj/+CMFlTUQWfqIxA7ttQ+UQO21BnSDm
 8IkRRZbRf1AI4iaf0OgBMwq97kZKl7X8Aiv0OGPLIPOUuQINBFo8IZ4BEACxbRsVx8WzwwKm
 0YUeVh5ZtL+IbHw0LXXU9dOyD0uTDPHx8LNMGtOrCQVfl8wIlIvzibxoqaEsXcqeZ38jRbZy
 f7HIhUU6Ek4mX763CQdm3EGGEwU3rSzJqyRwX9bhDq4w/1YJifIL9bcq/X5Z/jMDSsE0ii5x
 +RQKfUWoM3PeVTrKcc/mdyR3LVS3v5rpdN1aShn+ORfs6rFSkP71ZaC694ppI2nOo/tNe/wk
 PgBMCVgFa4QIMQKuGz7yiTwuHwjqvmn46wvG3nW+/NjOgfssKH2MwpukIjK+/gqdSjcpi4dH
 18gBAwP89RUzXru3EDVvcMQ36ieD10sq/oQH58JRvLoLYVtl5uGp64+6sT53dfNaHdjVIHEh
 3SNwwfGxlUZE/78Vpj26cle0QC8jFA5E/1xO8UaO60Fo1loJZb4iNiF2J87j0n+C5RKYL1Yg
 Zj91jF/JfFYRDayrXq+gOdZjFRSSgw+0+pjwZhXTf5PQUMQOqEMU11Z/ePrbMm6O8dj5xTXI
 +2tfag0bkFwfGjvv4pSFLHXwKlB7r5Cuza+KiAoBQEYRp0YOonX4BpKoP8PhW16wz8XGLE4z
 CfDOzq7bHDzgwISetUaS8HhGWNS16uL+7rWd9Ntcj8Mb84/IegOYHIRHFts/zwiyuMmQki6W
 B12H0ITgkfCU/d4nxbLcZQARAQABiQI8BBgBCAAmFiEEwO1R4Lve98qHHkUlflDxS081CzcF
 Alo8IZ4CGwwFCQeGH4AACgkQflDxS081CzdwnA//cw/6jcrsk8bIr9R+QbBdlUXKwPQ/milj
 7aoY8r5wG8oBlzJNPjChiaFPB878hsrPEMuwpARn0MLoxoEtA7cC6rXJq9Go71/xsTyikpSv
 nkZNljeWdC0av30eo7byE6kMPvCUiWq6MaY2GkV8vfvuYCEQLdIzqIv1P91R9qWGcV1vE4Gn
 JHdXYw6fvH7A8Hg5heq8EEhZSz4+mYuyygpCBhs9j/loERWULeCTI0E4PmjrcLFGxAD6ulte
 RUzZke+8/s4z6uPLyrX//On0G0ftwvZ6syU7xF7dARFgvTErZeiRoMle8D3EMMZsydSz7/v2
 edhbGB1yZaIqV5012MZQFv8HoqideriJ7C/F+gsvPuyBb/7UNYSVO4qLH5n+x7QFXWaaCmYL
 1AujbYbq7lz7avp2eL1QFRC6sTdouYlb9zMMF9ERCTW7G84xh2d6xHOD5SGFgGTDaeICKpr3
 jmNVhifiZAXgAWfZjhS25rpalyPBPGBF5yJCGyuKZdpuz2ltt83HkHyby3xuzbnyjNHMwsrk
 1i3jRmJQ9pa8S9F/+502pNXwFbWKUScOwrwpCsetnsfQyDsGS6DmWBM7Zt8hBW/ALas9a85T
 4rKiYBlqnYbLVDgf8CeoXWA3cNjf5lstMfMWzCSzG16v5emcXGFsR1owvo16p5DlCl2eKpYL Cvk=
Message-ID: <08e09ea4-20d0-277d-8919-4e3d4387699c@ctindustries.net>
Date: Wed, 13 Mar 2019 08:36:37 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0)
 Gecko/20100101 Thunderbird/60.5.3
MIME-Version: 1.0
In-Reply-To: <CAEZPtU5r9oFDdDj_81+u+a2ZUQk3EjQ8FK1TvOnsODHGJUUVqw@mail.gmail.com>
Content-Type: multipart/alternative;
 boundary="------------CBE04BCA57C28820921D6A16"
Content-Language: en-US
Feedback-ID: 2192m:2192ahkZ5OI:2192sitGAit-g7
Subject: Re: [PHP-DEV] On fixing DOMNameSpaceNode and DOM NS API Inconsistency
 Problems
From: rrichards@ctindustries.net (Rob Richards)

--------------CBE04BCA57C28820921D6A16
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

There were specific reasons for it tho I can't recall all the details
off the top of my head. Part of it related to the underlying structures
of attributes and namespace nodes not being the same and cannot be user
interchangeable in the code, unless only inspected at the nodeptr level.
Trying to manipulate namespace nodes as attributes will cause memory
corruption so they need to be handled differently. I don't remember if
this was the only reason or there was more for the special class but
while I agree with basic DOM usage no one should ever need to be dealing
directly with namespace declarations but once you get into more complex
situation, especially that involve xpath, xslt, canonicalization and
even interoperability this starts coming into play and is quite
important to deal with namespace declarations directly.

Also some of the examples in previous messages are way overly simplistic
so the conclusions are not 100% accurate. i.e. removeAttributeNS. I am
assuming that its just due to overly aggressive namespace reconciliation
happening and removing the declaration since its no longer in use. With
a nested document which uses the namespace in child nodes I would expect
the namespace declaration to remain, tho possible it may be moved to the
first child using it (been a while since I looked at that code).

My recommendation would be to tread lightly here, look at every method
any changes to namespace declaration nodes would touch and make sure
they are all handle properly otherwise you run the risk of introducing
memory corruption issues which is far worse than some inconsistency for
supposedly unused and unneeded functionality ;)

BTW I have recently started looking at the libxml code base again and
considering getting more involved here again, assuming the toxicity
level has gone done from the past. Not sure this is the first thing I
would look at but feel free to shoot over any questions.

Rob

On 3/12/19 1:50 AM, Pierre Joye wrote:
> Hi,
>
> Only adding Rob just in case :)
>
> best,
>
> On Wed, Mar 13, 2019, 5:54 AM Benjamin Eberlei <kontakt@beberlei.de
> <mailto:kontakt@beberlei.de>> wrote:
>
>     Hi everyone,
>
>     While looking for things to work on in php-src my friend Thomas
>     pointed me
>     to a peculiar special case in ext/dom that leads to massive
>     inconsistency
>     problems in the API.
>
>     There is an undocumented class DOMNameSpaceNode that gets returned
>     from
>     DOMElement::getAttributeNode(NS) if you select an attribute that
>     represents
>     a namespace (xmlns). This special case is intentionally handled in the
>     code, contrary to Pythons DOM Extension which doesn't do this and
>     contrary
>     to the DOM Specification, which does not have a special
>     DOMNameSpaceNode.
>     Its all DOMAttr.
>
>     Problematically DOMNameSpaceNode doesn't extend from DOMAttr, you
>     cannot
>     pass this class to DOMElement::removeAttributeNode(DOMAttr $attr):
>
>         Fatal error: Uncaught TypeError: Argument 1 passed to
>     DOMElement::removeAttributeNode() must be an instance of DOMAttr,
>     instance
>     of DOMNameSpaceNode given
>
>     Code example here: https://3v4l.org/jkC5s
>
>     In addition the DOMNameSpaceNode renames all properties compared to
>     DOMAttr, clearly violating the interface documented here
>     http://php.net/manual/de/domelement.getattributenode.php
>
>     Two potential fixes come to mind:
>
>     1. Have DOMNameSpaceNode extend DOMAttr - maybe this was the
>     originally
>     intended behavior, becuase the properties are all named differently?
>     2. Have DOMElement::removeAttributeNode accept also a DOMNameSpaceNode
>     (3.) Remove the non standard DOMNameSpaceNode class and use a DOMAttr
>     instead
>
>     I think approach #1 is the right one from a BC perspective and
>     also fixing
>     the DOM API to be more consistent with the standard.
>
>     But I am also not opposed to #3 in the longer term, looking at Github
>     DOMNamespaceNode in the open source world is only used for one apc
>     test, in
>     PHP Compat and when dumping it in Symfony Var Dumper. I imagine
>     its more
>     deeply used in closed source code working deeply with XML.
>
>     The second problem that this inconsistency creates is
>     DOMElement::removeAttributeNS($namespaceUri, $localName). When you
>     want to
>     use it to remove a namespace attribute (xmlns). By default the xmlns
>     namespace has the uri http://www.w3.org/2000/xmlns/. Hence removing
>     xmlns:$name namespace would expected to be:
>
>     DOMElement::removeAttributeNS("http://www.w3.org/2000/xmlns/",
>     "foo") //
>     removes xmlns:foo
>
>     But thats not how it works, there is special code implemented that
>     requires
>     you to pick the URI from xmlns:foo value:
>
>     <root xmlns:foo="urn:foo" />
>
>     DOMElement::removeAttributeNS("urn:foo", "foo");
>
>     But if your node has an attribute in this namespace with the same
>     name, it
>     gets removed as well:
>
>     <root xmlns:foo="urn:foo" foo:foo="bar" />
>
>     Would incorrectly become this, removing 2 attributes:
>
>     <root />
>
>     This is a bug that cannot be fixed in a BC way. I have no clue how
>     to fix
>     this completly broken behavior in a good way. I propose to break
>     BC here
>     and requiring "http://www.w3.org/2000/xmlns/" as namespaceURI from
>     PHP 8.0
>     to delete a namespace (xmlns) attribute.
>
>     Should I put both issues into an RFC or are these rather bugs that
>     can be
>     fixed without an RFC?
>
>     greetings
>     Benjamin
>


--------------CBE04BCA57C28820921D6A16--