Opened 9 years ago

Closed 5 years ago

Last modified 5 years ago

#2414 closed defect (wontfix)

coredump of python by a template extending itself

Reported by: chukharev@… Owned by: SmileyChris
Component: Template system Version:
Severity: normal Keywords:
Cc: karsten@…, crucialfelix@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I'm just starting with Django. When I made by a mistake so that a template jobs/base.html extended inself instead of intended base.html
(one level up), the development server died and I got a python.core file. I believe this is a bug in python interpreter, but I think that
django can detect the situation triggering this bug and present a resonable message. I also hope that django developers can extract
a small reproducible case for python developers...

If jobs/base.html starts with

{% extends "base.html" %}

everything works OK. If it starts with

{% extends "jobs/base.html" %}

the coredump happens.

Additional information:

$ uname -a
FreeBSD h33.h33.erkki.ton.tut.fi 6.1-RC FreeBSD 6.1-RC #0: Fri Apr 21 12:34:43 EEST 2006     root@h33.h33.erkki.ton.tut.fi:/usr/obj/usr/src/sys/H33  i386
$ pkg_info -I '*django*'
py24-django-devel-20060704 High-level Python Web framework

BTW, thanks for great software!

Attachments (2)

2414.diff (7.4 KB) - added by SmileyChris 6 years ago.
fix with tests
2414.2.diff (5.4 KB) - added by SmileyChris 5 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Changed 9 years ago by adrian

Oooh, interesting bug. Thanks for reporting!

comment:2 Changed 9 years ago by adrian

  • Status changed from new to assigned

comment:3 Changed 8 years ago by Karsten W. Rohrbach <karsten@…>

  • Cc karsten@… added

Could this be a stack size problem?

On FreeBSD, the default stack size is configured smaller than in most other distributions. This already bites on a regular basis with Zope installations. (e.g. things like http://tomster.org/blog/archive/2006/09/27/size-does-matter)

From a current FreeBSD port /usr/ports/lang/python24/Makefile:

.if defined(WITHOUT_HUGE_STACK_SIZE)
CFLAGS+=                -DTHREAD_STACK_SIZE=0x20000
.else
CFLAGS+=                -DTHREAD_STACK_SIZE=0x100000
.endif # defined(WITHOUT_HUGE_STACK_SIZE)                                       

comment:4 Changed 8 years ago by Karsten W. Rohrbach <karsten@…>

Addition: default behaviour was "WANT_HUGE_STACK_SIZE" (port version 1.128) and that has been changed in one of the latest port versions. (The default was set for the small stack size and is set for the big stack now in port version 1.155)

comment:5 Changed 8 years ago by SmileyChris

Can anyone confirm this problem or provide additional details?

comment:6 Changed 8 years ago by adrian

#3493 was a duplicate.

comment:7 Changed 8 years ago by adrian

  • Triage Stage changed from Unreviewed to Accepted

comment:8 Changed 8 years ago by Paul Bx <pb@…>

For what it's worth, my core dump report in #3493 was on FreeBSD as well.

comment:9 Changed 7 years ago by PhiR

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

Django 0.96 raises RunTimeError "maximum recursion depth exceeded" with python 2.4.4 undex linux 2.6.

comment:10 Changed 7 years ago by Simon Litchfield <simon@…>

  • Patch needs improvement set
  • Resolution worksforme deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Accepted to Design decision needed

I hate to keep reopening tickets but this bug still exists. I've ran into it on at least two separate occasions.

A common use case is extending the admin interface. For example, the homepage - say, adding a custom footer to the admin homepage. One might expect to create a file admin/index.html with {% extend 'admin/index.html' %} and a {% block footer %}. Not only does this not work but it also core dumps Django.

Simple fix is the template loader should *always* exclude itself from it's list of search paths when loading the template to be extended. This fixes the core dump bug aswell as providing greater flexibility. If anyone is interested I will submit the patch.

comment:11 Changed 7 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to Accepted

Since a template has no concept of source location (it doesn't have to come from a file, after all), the proposed solution won't work. We'll have to keep some kind of hash of the contents that's not too expensive to compute.

Worth doing if somebody wants to come up with a patch, although the core dump isn't a Django bug: pure Python code should never, ever dump core, so something is misbehaving very badly here.

comment:12 Changed 7 years ago by crucialfelix@…

Patch attached does fix it. I'm not sure but I think it wouldn't matter if it was non-file loaded. If it IS file loaded and the paths are identical, this throws an exception.

if you DO wish to extend the {parent template dir}/template then a custom extends could be written.

Index: /Users/crucial/Sites/sustainall/django/template/loader_tags.py
===================================================================
--- /Users/crucial/Sites/sustainall/django/template/loader_tags.py (revision 7774)
+++ /Users/crucial/Sites/sustainall/django/template/loader_tags.py (working copy)
@@ -62,6 +62,7 @@

return parent # parent is a Template object

try:

source, origin = find_template_source(parent, self.template_dirs)

+ assert self.source[0].name != origin.name, TemplateSyntaxError("Template '%s' cannot extend itself" % origin.name)

except TemplateDoesNotExist:

raise TemplateSyntaxError, "Template %r cannot be extended, because it doesn't exist" % parent

else:

comment:13 Changed 7 years ago by crucialfelix

resubmitting with code formatting.
this was against newforms-admin branch but I don't think that matters in this case.

Index: /Users/crucial/Sites/sustainall/django/template/loader_tags.py
===================================================================
--- /Users/crucial/Sites/sustainall/django/template/loader_tags.py	(revision 7774)
+++ /Users/crucial/Sites/sustainall/django/template/loader_tags.py	(working copy)
@@ -62,6 +62,7 @@
             return parent # parent is a Template object
         try:
             source, origin = find_template_source(parent, self.template_dirs)
+            assert self.source[0].name != origin.name, TemplateSyntaxError("Template '%s' cannot extend itself" % origin.name)
         except TemplateDoesNotExist:
             raise TemplateSyntaxError, "Template %r cannot be extended, because it doesn't exist" % parent
         else:

comment:14 Changed 7 years ago by crucialfelix@…

  • Cc crucialfelix@… added

adding self for cc

comment:15 Changed 7 years ago by crucialfelix@…

sorry, the patch I submitted previously does NOT work in all cases. an ExtendsNode has no source ...
I need to look at it further.

Changed 6 years ago by SmileyChris

fix with tests

comment:16 Changed 6 years ago by SmileyChris

  • Has patch set
  • Patch needs improvement unset

comment:17 Changed 6 years ago by SmileyChris

  • Owner changed from nobody to SmileyChris
  • Status changed from reopened to new

comment:18 Changed 5 years ago by adamnelson

  • Triage Stage changed from Accepted to Ready for checkin

comment:19 Changed 5 years ago by SmileyChris

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

This doesn't apply to trunk at the moment. The patch should still be sound, but some minor changes may need to be made to work with the new RenderContext changes.

Changed 5 years ago by SmileyChris

comment:20 Changed 5 years ago by SmileyChris

  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Accepted to Ready for checkin

Updated patch.

One change this patch makes is to change a TemplateSyntaxError to an ExtendsError, since it was in the module but never used. This meant changing a regression test (to expect that specific exception class). If that's a problem, it's easy to switch back.

comment:21 Changed 5 years ago by jacob

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

I'm going to wontfix this. On most machines -- including everywhere I've tried this -- you'll get a "maximum recursion depth exceeded" error. This is the correct error, and the same one you'd get with any Python function that unconditionally called itself.

If Python's dumping core that's a Python bug. We shouldn't be working around it; the problem needs to be fixed at the source.

comment:22 Changed 5 years ago by jacmkno

I'm created a branch of this ticket because per the main description it's ok to set it to wontfix, but there is still a feature missing to override the admin templates for all the apps.

Note: See TracTickets for help on using tickets.
Back to Top