#169 Missing events for end-element

closed-fixed
None
7
2002-07-02
2002-06-28
No

When the end-element handler is set but not the
start-element handler, the end-element events for end
tags are not reported; events are reported for empty
elements.

I've attached a test case that demonstrates this problem.

Discussion

  • Fred L. Drake, Jr.

    Logged In: YES
    user_id=3066

    I've attached a minimal patch to make Expat generate the
    required events. The attached test passes when this patch
    has been applied as well. The tests of the Python bindings
    that originally uncovered this bug also pass with this
    change to xmlparse.c.

     
  • Fred L. Drake, Jr.

    Logged In: YES
    user_id=3066

    I've gone ahead and checked in the patches as lib/xmlparse.c
    1.46 and tests/runtests.c 1.23.

     
  • Fred L. Drake, Jr.

    • status: open --> closed-fixed
     
  • Karl Waclawek

    Karl Waclawek - 2002-06-28

    Logged In: YES
    user_id=290026

    Should it not now be possible to change

    ...
    if (endElementHandler && tag->name.str) {
    ...

    to

    ...
    if (endElementHandler) {
    ...

    since the code should guarantee that the correct
    tag is accessed, which means that tag->name.str != NULL?

    Karl

     
  • Fred L. Drake, Jr.

    Logged In: YES
    user_id=3066

    Ok, I've thought about that a bit, and the tests pass with
    that change, but I think that means we're missing a test.

    I suspect the current implementation largely assumes that
    once we start parsing, the set of handlers will not grow;
    while this may be reasonable application behavior, this is a
    bad assumption to make in library code.

    Consider this: some processor watches for a processing
    instruction, and *adds* handlers based on that. Suppose
    that it adds start/end element handlers where it had none
    before, and the document looks like this:

    <doc>
    <?mytool start?>
    <inner/>
    </doc>

    If we make the change you suggest, we could be passing bad
    data (NULL for the tagName) to the endElementHandler for the
    doc element, though all will be fine for the inner element.
    With the extra check in there, it means we don't call the
    endElementHandler for the doc element, which, while a bug,
    means we're preserving existing behavior instead of passing
    NULL (not documented as a valid value for the tagName).

    I think we should leave the extra check in for now, but
    should consider always setting up the TAG structure, even
    though it means extra work for the rare application that
    doesn't use either the start or end element handler. The
    data is still potentially needed for any application that
    add an endElementHandler after processing starts.

    There are probably similar cases for paired handlers
    elsewhere in the code as well.

     
  • Karl Waclawek

    Karl Waclawek - 2002-06-29

    Logged In: YES
    user_id=290026

    I agree with your analysis, also with the need
    for checking other handler pairs.

    One quick idea, without looking at the source:

    If we remove this check:
    ...
    if (startElementHandler || endElementHandler) {
    ...
    then the tag structure gets always set up and we
    would not need the extra check for endElementHandler.
    At the cost of extra processing time, but how
    often does one *not* set any of the element handlers?

    Karl

     
  • Karl Waclawek

    Karl Waclawek - 2002-06-30

    Logged In: YES
    user_id=290026

    I attached a patch (based on your patch - xmlparse.c 1.46),
    that always sets up the tag structure, as suggested.
    Additionally, I made sure that storeAtts doesn't get
    called twice - I moved it under if (startElementHandler) ...

    That way we should have correct behaviour, even if the
    endElementHandler gets set after the startElementHandler
    was called. Since tag->name.str should now always be
    non-NULL, I removed the corresponding check.

    Karl

     
  • Karl Waclawek

    Karl Waclawek - 2002-06-30

    Patch that removes check for start/endElementHandler

     
  • Fred L. Drake, Jr.

    Logged In: YES
    user_id=3066

    Karl, the additional patch looks good to me; go ahead and
    check it in, and I'll finish up the hard-tab removal task.

    Re-close the bug report when you're ready.

     
  • Fred L. Drake, Jr.

    • priority: 5 --> 7
    • assigned_to: fdrake --> kwaclaw
    • status: closed-fixed --> open-fixed
     
  • Karl Waclawek

    Karl Waclawek - 2002-07-02
    • status: open-fixed --> closed-fixed
     
  • Karl Waclawek

    Karl Waclawek - 2002-07-02

    Logged In: YES
    user_id=290026

    Additional patch checked in.

    Karl

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks