Edgewall Software

Opened 15 years ago

Closed 13 years ago

Last modified 10 years ago

#370 closed defect (fixed)

Regression: multiple py:match elements cause Exception 'list index out of range'

Reported by: Felix Schwarz <felix.schwarz@…> Owned by: hodgestar
Priority: major Milestone: 0.6.1
Component: Template processing Version: devel
Keywords: Cc: osimons, mmitar@…, leho@…

Description

We have a Genshi template that uses several stacked py:match expressions which worked with 0.5.1. Using the latest 0.6 trunk I get an exception when rendering this template with Genshi:

...
File "trac/web/chrome.py", line 832, in _strip_accesskeys
  for kind, data, pos in stream:
File "genshi/core.py", line 288, in _ensure
  for event in stream:
File "trac/web/chrome.py", line 821, in _generate
  for kind, data, pos in stream:
File "genshi/template/base.py", line 592, in _include
  for event in stream:
File "genshi/template/markup.py", line 344, in _match
  if test(event, namespaces, ctxt) is True:
File "genshi/path.py", line 129, in _test
  pos_queue = deque([(pos, cou, []) for pos, cou in stack[-1]])

Attachments (5)

pymatch_regression.py (630 bytes) - added by Felix Schwarz <felix.schwarz@…> 15 years ago.
Testcase
match_with_if (659 bytes) - added by Felix Schwarz <felix.schwarz@…> 15 years ago.
preliminary patch
simple_fix_bad_test.py (615 bytes) - added by Felix Schwarz <felix.schwarz@…> 15 years ago.
Actually my 'fix' is faulty, as this test case fails after my proposed modification
ticket370.diff (2.1 KB) - added by cmlenz 15 years ago.
Proposed fix
fail_with_more_complicated_conditions.patch (1.2 KB) - added by Felix Schwarz <felix.schwarz@…> 15 years ago.

Download all attachments as: .zip

Change History (26)

Changed 15 years ago by Felix Schwarz <felix.schwarz@…>

Testcase

comment:1 Changed 15 years ago by Felix Schwarz <felix.schwarz@…>

The problem exists since r1020 when soc2008-xpath was merged into trunk.

Changed 15 years ago by Felix Schwarz <felix.schwarz@…>

preliminary patch

comment:2 Changed 15 years ago by Felix Schwarz <felix.schwarz@…>

Ok, I attached some kind of patch - however I don't even pretend to understand what's happening there so probably it's the wrong way to fix the issue. However it works for me.

Changed 15 years ago by Felix Schwarz <felix.schwarz@…>

Actually my 'fix' is faulty, as this test case fails after my proposed modification

Changed 15 years ago by cmlenz

Proposed fix

comment:3 Changed 15 years ago by cmlenz

  • Status changed from new to assigned

I've added a patch that fixes both test cases in my testing. I would very much appreciate if more people could try this patch and let me know whether it breaks things elsewhere.

comment:4 Changed 15 years ago by cmlenz

  • Resolution set to fixed
  • Status changed from assigned to closed

Patch applied in [1097].

comment:5 Changed 15 years ago by rblank

The Trac test suite passes fine here with the patch (Linux, Python 2.6.4). I get one failure in the Genshi test suite, but it must be unrelated, as it is also present without the patch:

======================================================================
FAIL: Doctest: genshi.template.eval.Undefined
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.6/doctest.py", line 2145, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for genshi.template.eval.Undefined
  File "/home/joe/src/genshi/trunk/genshi/template/eval.py", line 219, in Undefined

----------------------------------------------------------------------
File "/home/joe/src/genshi/trunk/genshi/template/eval.py", line 230, in genshi.template.eval.Undefined
Failed example:
    list(foo)
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python2.6/doctest.py", line 1241, in __run
        compileflags, 1) in test.globs
      File "<doctest genshi.template.eval.Undefined[2]>", line 1, in <module>
        list(foo)
      File "/home/joe/src/genshi/trunk/genshi/template/eval.py", line 277, in _die
        raise UndefinedError(self._name, self._owner)
    UndefinedError: "foo" not defined


----------------------------------------------------------------------
Ran 814 tests in 1.603s

FAILED (failures=1)

comment:6 Changed 15 years ago by rblank

Proposed fix for the failing test case:

  • genshi/template/eval.py

    diff --git a/genshi/template/eval.py b/genshi/template/eval.py
    a b  
    259259        self._name = name
    260260        self._owner = owner
    261261
     262    def __getattr__(self, name):
     263        if name == '__length_hint__':
     264            raise AttributeError()
     265        self._die()
     266
    262267    def __iter__(self):
    263268        return iter([])
    264269
     
    275280        """Raise an `UndefinedError`."""
    276281        __traceback_hide__ = True
    277282        raise UndefinedError(self._name, self._owner)
    278     __call__ = __getattr__ = __getitem__ = _die
     283    __call__ = __getitem__ = _die
    279284
    280285
    281286class LookupBase(object):

comment:7 Changed 15 years ago by rblank

Oh, this was #324. Sorry for the noise.

comment:8 Changed 15 years ago by cmlenz

So the patch from #324 works for you? It's a bit simpler. Don't have 2.6.2 for testing available right now.

comment:9 Changed 15 years ago by rblank

Yes, the simple fix works for me.

comment:10 follow-up: Changed 15 years ago by Felix Schwarz <felix.schwarz@…>

Thank you very much for the fix - however I found another issue which looks very similar. However we can work around the issue so for us it's not too important. Should I reopen the ticket or create a new one?

Changed 15 years ago by Felix Schwarz <felix.schwarz@…>

comment:11 in reply to: ↑ 10 Changed 15 years ago by cmlenz

Replying to Felix Schwarz <felix.schwarz@…>:

Thank you very much for the fix - however I found another issue which looks very similar. However we can work around the issue so for us it's not too important. Should I reopen the ticket or create a new one?

Hmm, I don't think you can match <div id="fnord" /> with the XPath pattern div//*[@id='fnord']. That will only look for elements with the "fnord" ID in all of the ancestors of a div, not the div itself. Probably that was a bug in the old XPath implementation.

comment:12 Changed 14 years ago by osimons

  • Cc osimons added
  • Resolution fixed deleted
  • Status changed from closed to reopened

[1097] involved breaking change for basic match handling, breaking simple matching rules. Test and output below is from running rev 1096, and it can easily be verified not to work with current Genshi:

In [1]: from genshi.template import MarkupTemplate

In [2]: print(MarkupTemplate("""<?xml version="1.0"?>
   ...: <root xmlns:py="http://genshi.edgewall.org/">
   ...:   <py:match path="foo/bar">
   ...:     <zzzzz/>
   ...:   </py:match>
   ...:   <foo>
   ...:     <bar/>
   ...:     <bar/>
   ...:   </foo>
   ...:   <bar/>
   ...: </root>""").generate())
<?xml version="1.0"?>
<root>
  <foo>
    <zzzzz/>
    <zzzzz/>
  </foo>
  <bar/>
</root>

Current wrong output is:

<root>
  <foo>
    <zzzzz/>
    <bar/>
  </foo>
    <zzzzz/>
</root>

This should likely provide the basis for a new def test_match_multiple_times3(self) test case...

comment:13 Changed 14 years ago by osimons

I forgot: Credit for discovery & test goes to Ozzi Lee reporting this to IRC + mailing list.

comment:14 Changed 14 years ago by hodgestar

  • Milestone changed from 0.6 to 0.6.1

comment:15 Changed 14 years ago by mmitar@…

Is there a workaround?

comment:16 Changed 14 years ago by Mitar

  • Cc mmitar@… added

comment:17 Changed 13 years ago by hodgestar

  • Owner changed from cmlenz to hodgestar
  • Status changed from reopened to new

comment:18 Changed 13 years ago by hodgestar

  • Resolution set to fixed
  • Status changed from new to closed

It looks like it was an out-by-error in the one-line fix committed in r1097. The current match template also needs to have test(tail[0]) run so that it can update its state. Fix (with new test -- thanks osimons and Ozzi Lee!) committed in r1170 and merged to 0.6.x in r1171.

comment:19 Changed 10 years ago by lkraav <leho@…>

  • Cc leho@… added

comment:20 Changed 10 years ago by lkraav <leho@…>

I'm able to get this same error with this Trac site.html template, on Genshi 0.7

<html xmlns="http://www.w3.org/1999/xhtml"
      xmlns:xi="http://www.w3.org/2001/XInclude"
      xmlns:py="http://genshi.edgewall.org/"
      py:strip="">
<py:if test="req.path_info.startswith('/ticket') and (not 'preview' in req.args)">
  <py:match path="div[@id='main']">
    <div id="main">
      ${select("*")}
      <div class="sidebar sidebar-ticket">${select("//div[@id='ticket']/table[@class='properties']")}</div>
      <div py:match="//div[@id='ticket']/table[@class='properties']" py:strip=""></div>
    </div>
  </py:match>
  <div py:match="div[@id='ctxtnav']" py:strip=""></div>
  <div py:match="div[@id='altlinks']" py:strip=""></div>
</py:if>

I'm looking to move the table from one div to another. Nested py:match statement generates this error.

If I move the strip match outside of the original element, then both tables disappear from different parts of the document. I don't think this should be happening based on the specifically restricted XPath expression I'm using.

Reopen?

comment:21 Changed 10 years ago by hodgestar

As described in http://genshi.edgewall.org/wiki/Documentation/xml-templates.html#id5, match templates form a sort of processing pipeline. I'm not sure what nested py:match statements should mean and I'm a bit surprised they don't raise an error.

If you feel something odd is happening with the unnested version, could you open a ticket for that with a minimal example of the oddly behaving template and a small Python script that calls it?

Note: See TracTickets for help on using tickets.