Bug #80332 [NEW]: Completely broken array access functionality with DOMNamedNodeMap

From: Date: Sat, 07 Nov 2020 07:00:01 +0000
Subject: Bug #80332 [NEW]: Completely broken array access functionality with DOMNamedNodeMap
Groups: php.bugs 
Request: Send a blank email to php-bugs+get-230171@lists.php.net to get a copy of this message
From:             asmqb7 at gmail dot com
Operating system: Debian/likely all OSes
PHP version:      7.4.12
Package:          DOM XML related
Bug Type:         Bug
Bug description:Completely broken array access functionality with DOMNamedNodeMap

Description:
------------
(Apologies for the multiple inline explanatory pseudo-code samples, but
I was unsure how to best outline the usability impact of this bug
without them. See also the complete test script.)

At first glance, it appears to be possible/acceptable to manipulate DOM
element attributes via direct array access. For example, the following
code...

  $doc = new DOMDocument;
  $doc->loadHTML('<a href="https://example.com">hi!</a>');
  $a = (new DOMXPath($doc))->query('//a')[0];
  $a->attributes['href']->nodeValue = 'https://example.net';
  print $doc->saveHTML();
	
...appears to work, producing:

  <a href="https://example.net">hi!</a>

An inexperienced or non-oriented developer (and I was both of those
yesterday afternoon, with regards to DOMDocument) might stumble on the
above approach through blind experimentation and, *after observing that
a seemingly complex manipulation produces correct end-to-end results
without throwing any errors*, conclude that DOMNamedNodeMap permits and
supports direct array access, similar to how direct DOMNode->nodeValue
manipulation works elsewhere.

The fact that this approach is straightforward and intuitive - and
appears to work - may only serve to lengthen the time the developer goes
around in circles suspecting everything but the array access code, when
things go sideways in extremely confusing ways.

For example, introducing a second attribute...

  ...
  $doc->loadHTML('<a href="https://example.com"
class="myclass">hi!</a>');
  ...
  print $a->attributes['class']->nodeName."\n";
  $a->attributes['class']->nodeValue = 'class2';
  print $doc->saveHTML();

...will print:

  href
  <a href="class2">hi!</a>

This is because (insert "SEVERAL HOURS LATER" graphic here) /whichever
key you attempt to read will return the first DOMAttr/, including
nonexistent keys:

  ...
  $doc->loadHTML('<a href="https://example.com">hi!</a>');
  ...
  print $a->attributes['IDoNotExist']->nodeName."\n";
  ...

This produces:

  href

I am extremely appreciative DOMDocument is not responsible for holding
together the fabric of reality.

On a more serious note, it appears that DOMNamedNodeMap implements
Iterable in the most literal sense - as an iterator:

  ...
  $doc->loadHTML('<a href="https://example.com" class="myclass"
id="myid">hi!</a>');
  ...
  foreach ($a->attributes as $attr) {
    print $attr->nodeName.': '.$attr->nodeValue."\n";
  }

This produces:
  
  href: https://example.com
  class: myclass
  id: myid

And modifying the foreach to try to write back by reference (&)...

  ...
  ...
  foreach ($a->attributes as &$attr) {
    ...
  ...

...doesn't work:

  Fatal error: Uncaught Error: An iterator cannot be used with foreach
by reference

So everything sorta makes sense now; DOMNamedNodeMaps are iterators.

But this does not completely square off the fact that it's possible to
read the first attribute in a DOMNamedNodeMap by passing in any string.
For conciseness, this area is the sole focus of the test script.

The DOMNamedNodeMap-as-iterators implementation detail is not documented
anywhere. I feel that knowing this may have been useful, but only
because the implementation is broken.

(NB. Submission-related trivia: Thanks to the fact that the web-bugs
repo is online, I was able to change the http:// URLs above to https://
so is_spam() didn't find 5 'http://' substrings and
return true :D)

Test script:
---------------
<?php

  $doc = new DOMDocument;
  
  $doc->loadHTML('<span attr1="value1"
attr2="value2"></span>');
  
  $x = new DOMXPath($doc);
  $span = $x->query('//span')[0];
  
  print "Node name: {$span->nodeName}\n";

  print "Attribute [0] name: {$span->attributes[0]->nodeName}\n";
  print "Attribute [0] value: {$span->attributes[0]->nodeValue}\n";

  print "Attribute 'hi' name:
{$span->attributes['hi']->nodeName}\n";
  print "Attribute 'hi' value:
{$span->attributes['hi']->nodeValue}\n";

  print "Attribute 'attr2' name:
{$span->attributes['attr2']->nodeName}\n";
  print "Attribute 'attr2' value:
{$span->attributes['attr2']->nodeValue}\n";


Expected result:
----------------
The two logical and safe results I would expect would be either

1) Any array access attempt to print a warning or throw an
error/exception

2) For array accesses of
  2a) extant attributes to return their associated DOMAttr node
  2a) non-existing attributes to print a warning or throw an
error/exception

If it is decided that (2) is not appropriate - for whatever reason - I
would emphasize my support for the implementation of (1) as soon as
possible, to ensure that any code erroneously using array accesses
smashes into an appropriate brick wall so it can be fixed.

Actual result:
--------------
Node name: span
Attribute [0] name: attr1
Attribute [0] value: value1
Attribute 'hi' name: attr1
Attribute 'hi' value: value1
Attribute 'attr2' name: attr1
Attribute 'attr2' value: value1


-- 
Edit bug report at https://bugs.php.net/bug.php?id=80332&edit=1
-- 
Fix committed:                    https://bugs.php.net/fix.php?id=80332&r=fixed
Fixed in release:                 https://bugs.php.net/fix.php?id=80332&r=alreadyfixed
Need backtrace:                   https://bugs.php.net/fix.php?id=80332&r=needtrace
Need Reproduce Script:            https://bugs.php.net/fix.php?id=80332&r=needscript
Try newer version:                https://bugs.php.net/fix.php?id=80332&r=oldversion
Not developer issue:              https://bugs.php.net/fix.php?id=80332&r=support
Expected behavior:                https://bugs.php.net/fix.php?id=80332&r=notwrong
Not enough info:                  https://bugs.php.net/fix.php?id=80332&r=notenoughinfo
Submitted twice:                  https://bugs.php.net/fix.php?id=80332&r=submittedtwice
register_globals:                 https://bugs.php.net/fix.php?id=80332&r=globals
PHP version support discontinued: https://bugs.php.net/fix.php?id=80332&r=phptooold
Daylight Savings:                 https://bugs.php.net/fix.php?id=80332&r=dst
IIS Stability:                    https://bugs.php.net/fix.php?id=80332&r=isapi
Install GNU Sed:                  https://bugs.php.net/fix.php?id=80332&r=gnused
Floating point limitations:       https://bugs.php.net/fix.php?id=80332&r=float
No Zend Extensions:               https://bugs.php.net/fix.php?id=80332&r=nozend
MySQL Configuration Error:        https://bugs.php.net/fix.php?id=80332&r=mysqlcfg


Thread (4 messages)

« previous php.bugs (#230171) next »