Zope 2.4.2 DTML Method Bug
One of my products exposed a bug in the 2.4.2 version of DTMLMethod.py. It is minor and can be worked around, but I wanted to point it out: Line 203 of DTMLMethod.py now contains: del self.__dict__['validate'] which is part of a try...finally statement. It seems that the "validate" key is not always present in the object at that point, specifically if you recursively call an object in a different context then it was originally called. as in: <dtml-with name="something"> <dtml-return name="this"> </dtml-with> This piece of code resulted in a KeyError on "validate" in my product code, which had previously worked fine. Perhaps another try statement should be wrapped around this del statement? /---------------------------------------------------\ Casey Duncan, Sr. Web Developer National Legal Aid and Defender Association c.duncan@nlada.org \---------------------------------------------------/
Casey Duncan wrote:
One of my products exposed a bug in the 2.4.2 version of DTMLMethod.py. It is minor and can be worked around, but I wanted to point it out:
Line 203 of DTMLMethod.py now contains:
del self.__dict__['validate']
which is part of a try...finally statement.
It seems that the "validate" key is not always present in the object at that point, specifically if you recursively call an object in a different context then it was originally called. as in:
<dtml-with name="something"> <dtml-return name="this"> </dtml-with>
This piece of code resulted in a KeyError on "validate" in my product code, which had previously worked fine. Perhaps another try statement should be wrapped around this del statement?
This happens when a dtml method is reentrant. The fix needs to be a bit deeper than what you describe above, otherwise you'll potentially lose the "validate" attribute of the dtml method object, and you'll get strange errors. I've put this into the new collector, with a patch: http://new.zope.org/Collector/4 -- Steve Alexander Software Engineer Cat-Box limited
Casey Duncan wrote:
One of my products exposed a bug in the 2.4.2 version of DTMLMethod.py. It is minor and can be worked around, but I wanted to point it out:
Line 203 of DTMLMethod.py now contains:
del self.__dict__['validate']
which is part of a try...finally statement.
It seems that the "validate" key is not always present in the object at that point, specifically if you recursively call an object in a different context then it was originally called. as in:
<dtml-with name="something"> <dtml-return name="this"> </dtml-with>
This piece of code resulted in a KeyError on "validate" in my product code, which had previously worked fine. Perhaps another try statement should be wrapped around this del statement?
Hi Casey, I think that is fixed but I'm not positive that its in 2.4.2 -- I know its on my 2_4 branch; I think we just barely missed this for 2.4.2 -- I see the change going into the log about a week later. I'll ask Brian if we're going to put out a 2.4.3 to include the fix.
Matthew T. Kromer wrote:
I think that is fixed but I'm not positive that its in 2.4.2 -- I know its on my 2_4 branch; I think we just barely missed this for 2.4.2 -- I see the change going into the log about a week later.
I'll ask Brian if we're going to put out a 2.4.3 to include the fix.
Matt, Please see my report in the new Collector. The patch at the end of this email is better than the one in the Collector, and also not mangled by stx :-) Your fix in the trunk does only fixes the symptom. It does not address the problem of DTML Methods not being reenterant. Your fix is to wrap in a try-except the act of removing validate from the __dict__ of the dtml method object: try: del self.__dict__['validate'] except: pass If you have a dtml method object that calls itself (that is, __call__ is called when a __call__ is already executing), then the validate attribute will be removed before the outer call is finished. Here's my patch. As a bonus, we get rid of an unqualified except: statement. This is a patch against the trunk. *** DTMLMethod.py.orig Fri Nov 2 11:45:54 2001 --- DTMLMethod.py Fri Nov 2 11:47:45 2001 *************** *** 180,186 **** security=getSecurityManager() security.addContext(self) ! self.__dict__['validate'] = security.DTMLValidate try: if client is None: --- 180,192 ---- security=getSecurityManager() security.addContext(self) ! ! if self.__dict__.has_key('validate'): ! fist_time_through=0 ! else: ! self.__dict__['validate'] = security.DTMLValidate ! first_time_through=1 ! try: if client is None: *************** *** 200,207 **** finally: security.removeContext(self) ! try: del self.__dict__['validate'] ! except: pass have_key=RESPONSE.headers.has_key if not (have_key('content-type') or have_key('Content-Type')): --- 206,213 ---- finally: security.removeContext(self) ! if first_time_through: ! del self.__dict__['validate'] have_key=RESPONSE.headers.has_key if not (have_key('content-type') or have_key('Content-Type')): You could also only add the dtml method to the security context on the first time through, although I think this would break the detection of excessive recursion. So, I haven't done that. -- Steve Alexander Software Engineer Cat-Box limited
On Friday 02 November 2001 06:51 am, Steve Alexander allegedly wrote:
Matthew T. Kromer wrote:
I think that is fixed but I'm not positive that its in 2.4.2 -- I know its on my 2_4 branch; I think we just barely missed this for 2.4.2 -- I see the change going into the log about a week later.
I'll ask Brian if we're going to put out a 2.4.3 to include the fix.
Matt,
Please see my report in the new Collector. The patch at the end of this email is better than the one in the Collector, and also not mangled by stx :-)
Your fix in the trunk does only fixes the symptom. It does not address the problem of DTML Methods not being reenterant.
[snip]
Here's my patch. As a bonus, we get rid of an unqualified except: statement.
Steve, Your fix seems correct to me, just swallowing the exception is probably a subtle security hole at worst at best it is just sweeping things "under the rug" as it were. It might be worth stating that constructs such as: try: foo except: pass Smack of bad form and should be avoided at all costs... They can make debugging a nightmare. It would be nice if this patch could make it into 2.4.3b3. Thanks Steve! /---------------------------------------------------\ Casey Duncan, Sr. Web Developer National Legal Aid and Defender Association c.duncan@nlada.org \---------------------------------------------------/
Casey Duncan wrote:
It would be nice if this patch could make it into 2.4.3b3.
I just got a fresh CVS checkout, and Matt has committed this to CVS. I don't know if there will be a 2.4.3b release, (Brian? Matt?), but if there is, this patch will be in it. -- Steve Alexander Software Engineer Cat-Box limited
Steve Alexander wrote:
Casey Duncan wrote:
It would be nice if this patch could make it into 2.4.3b3.
I just got a fresh CVS checkout, and Matt has committed this to CVS.
I don't know if there will be a 2.4.3b release, (Brian? Matt?), but if there is, this patch will be in it.
I didn't really give other developers enough time to make changes they wanted to make but were putting off (not that I know of any, but that's the whole point) into the Zope-2_4-branch to cut a beta release today, so I'm going to do that on Monday. I dont think it will take more than 90 minutes or so to do, so Zope 2.4.3b1 should probably land about noon on Monday.
participants (3)
-
Casey Duncan -
Matthew T. Kromer -
Steve Alexander