190596 - Fragile assumptions about XML serialization in utility to import bugs from XML

Status: RESOLVED FIXED

(Bugzilla :: Bug Import/Export & Moving, defect)

Description

[Bob Kline]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.2) Gecko/20021120 Netscape/7.01
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.2) Gecko/20021120 Netscape/7.01

The code to import bugs from an XML document uses a bizarre approach to finding
the elements it's looking for, assuming it knows where the elements are by
position.  If the whitespace nodes contained in the serialization of the XML
document do not exactly match what the code naively expects, the bugs are not
imported correctly, or are not imported at all.  It's even possible to submit an
XML document which results in an amusing email report that "0.5 bug(s)
successfully moved from ...."

Reproducible: Always

Steps to Reproduce:
1. Create an XML bugzilla document with no extra whitespace nodes between elements.
2. Run importxml.pl.
3. Check your email.

Actual Results:  
No bugs were imported.

Expected Results:  
Imported the bugs.

This should really be a separate bug, assuming I'm right, but there should be
documentation of the requirements for the XML document used by importxml.pl to
import bugs.  I'll defer filing it to give folks a chance to tell me I just
wasn't looking in the right place for the documentation.  Saying I should have
used the source is not an appropriate substitute in this case, given the cryptic
approach used by the code in this utility for dealing with the data in the XML
document.

Comment 1

[Dave Miller [:justdave]]

yeah, docs bug already exists (bug 189154)

like any good XML, the requirements are in a DTD file. :)

http://bugzilla.mozilla.org/bugzilla.dtd

since whitespace between tags is supposed to be optional in XML, if a lack of
whitespace is killing it, we definitely have a problem....

Comment 2

[Dave Miller [:justdave]]

*** Bug 190598 has been marked as a duplicate of this bug. ***

Comment 3

[Bob Kline]

> like any good XML, the requirements are in a DTD file. :)
> 
> http://bugzilla.mozilla.org/bugzilla.dtd

I'm hoping that your smiley means that you are joking in this case.  Since this
DTD has not a single comment one would ordinarily assume that the
element/attribute names and DTD structural requirements convey everything the
programmer needs to know in order to generate useful XML documents for the
import utility.  Given that every child element except bug_id under 'bug' is
optional, according to the DTD, this would mean that the import utility would do
something useful if any of those elements are omitted, and that any guesses I
make about what should go in the attributes for the document-level element would
be acceptable, right? :->}

(Oops, I just inadvertantly created a duplicate of this bug with the refresh
button in another browser window; I'll suppress it if I have adequate
permissions.  That's one of the problems we had with our home-grown tracking
system we were hoping to eliminate by switching to Bugzilla.  Sigh.)

Comment 4

[Gervase Markham [:gerv]]

> Given that every child element except bug_id under 'bug' is
> optional, according to the DTD, this would mean that the import utility would 
> do something useful if any of those elements are omitted, 

No, it means that it's valid to export a bug with any number of the elements
present. This particular bug import utility might require them all to be present
before working; others (for example, Scarab's) may not. Who knows? 

I of course agree that our XML import is too fragile and should be fixed, but I
don't see anything wrong with our DTD.

> Oops, I just inadvertantly created a duplicate of this bug with the refresh
> button in another browser window

Does your browser not warn you about the resubmission of POST requests?

Gerv

Comment 5

[Bob Kline]

> No, it means that it's valid to export a bug with any number of the elements
> present. This particular bug import utility might require them all to be
> present before working; others (for example, Scarab's) may not. Who knows? 
> 
> I of course agree that our XML import is too fragile and should be fixed, but I
> don't see anything wrong with our DTD.

I think you missed the smileys.  The DTD is not the problem I was originally
referring to, it's the lack of documentation for the utility.  Dave jokingly
suggested that no further docs would be necessary, since we have a DTD (he was
kidding), and I was just playing along with the gag.

> Does your browser not warn you about the resubmission of POST requests?

Yes, sorry, that was my fault.  I apologize.  Won't happen again.

Comment 6

[Jacob Steenhagen]

<offtopic>
> Does your browser not warn you about the resubmission of POST requests?

That doesn't excuse us from detecting/attempting to prevent it, but that's a
different bug (bug 46843, to be exact)
</offtopic>

Comment 7

[Dave Miller [:justdave]]

Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.

Comment 8

[Frédéric Buclin]

AFAIK, this bug will be fixed by bug 285614. Greg, am I right?

Comment 9

[Greg Hendricks]

yes, check dependencies ;)

Comment 10

[Frédéric Buclin]

fixed by bug 285614