zopefoundation / Zope

Zope is an open-source web application server.
https://zope.readthedocs.io
Other
357 stars 101 forks source link

Products.PageTemplates (with chameleon): improper handling of (some) syntax errors #710

Open d-maurer opened 5 years ago

d-maurer commented 5 years ago

Some syntax errors are badly handled as the following transscript demonstrates:

>>> # ensure we use the `chameleon` template engine
>>> from zope.component import provideUtility
>>> from Products.PageTemplates.engine import Program
>>> provideUtility(Program)
>>> 
>>> from Products.PageTemplates.PageTemplate import PageTemplate
>>> pt = PageTemplate()
>>> pt.pt_edit(r'''
... <html><body>
... <div \
...    >
...    div with syntax error in start tag
...    <div>nested div</div>
... </div>
... </body></html>''',
... "text/html"
... )
>>> pt()
Traceback (most recent call last):
...
  File ...zope/pagetemplate/pagetemplate.py", line 129, in pt_render
    raise PTRuntimeError(str(self._v_errors))
zope.pagetemplate.pagetemplate.PTRuntimeError: ['Compilation failed', 'chameleon.exc.ParseError: Unexpected end tag.\n\n - String:     "</div>"\n - Filename:   <string>\n - Location:   (line 7: col 0)']

The trailing \ in the start tag of the outer div is clearly a syntax error. However, it is not flagged out as such. Instead the malformed start tag is silently ignored causing an error when the corresponding end tag is eventually reached. In more complex examples, the error may be reported even later which can make the analysis of the problem very expensive.

An initial analysis revealed: the syntax error is properly recognized by the "chameleon" parser. It tries to report it by calling a "visit" function with category "error". This tries to look up visit_error and finds it undefined. Therefore, it uses a default handling which ignores the tag.

icemac commented 5 years ago

@d-maurer Should this be fixed here in the Zope package or in zope.pagetemplate as the traceback seems to point to?

d-maurer commented 5 years ago

Michael Howitz wrote at 2019-10-11 06:36 -0700:

@d-maurer Should this be fixed here in the Zope package or in zope.pagetemplate as the traceback seems to point to?

I do not yet know: have not yet found time to analyse in detail. During the initial analysis, I observed that something was missing a "visit_error" but I forgot to note which type of object this was.

Do not give the traceback too much importance: it describes a followup problem; the primary problem (the syntax error) does not give a traceback - but simply ignores the tag (which of course destroys the XML structure and lead to the followup problem).

d-maurer commented 5 years ago

The error is actually in chameleon: chameleon.zpt.program.MacroProgram lacks a visit_error method.

Searching github (a few minutes ago) for chameleon caused Whao, you have triggered an abuse handling; try again later.

A few days ago, I hit another chameleon issue. My template contained:

<!--
...
<... tal:repeat="lv ..." ...>...<... tal:content="lv" .../>...</...>
-->

and rendering gave a NameError lv. Apparently, the tal:repeat was not executed but the tal:content was. I have not yet further analysed the issue (but simply removed the comment).

d-maurer commented 5 years ago

The error is actually in chameleon: chameleon.zpt.program.MacroProgram lacks a visit_error method.

The primary error is actually a missing visit_error in chameleon.parser.ElementParser.

dataflake commented 4 years ago

Dieter, was this fixed as part of Chameleon 3.7.0?

d-maurer commented 4 years ago

Jens Vagelpohl wrote at 2020-3-31 01:30 -0700:

Dieter, was this fixed as part of Chameleon 3.7.0?

I do not think so.

I filed "https://github.com/malthe/chameleon/issues/298" for this issue -- and it got the label "feature". No sign about work on it.

dataflake commented 4 years ago

FYI, this is still unfixed with Chameleon 3.8.1

d-maurer commented 8 months ago

The bug is not yet fixed in Chameleon = 4.2.0 (used by Zope 5.9.x).