Skip to content

Template loader problems with absolute paths #731

Open
@bdarnell

Description

@bdarnell

Several problems in Loader.resolve_path:

  • If RequestHandler.render() is passed an absolute path, relative paths in {%extends%} and friends don't work.
  • It uses startswith('/') instead of isabs()
  • Strange things happen if you use .. in an extends directive to break out of the template loader's root.

From the mailing list:
In my project, template folder is not include the template root path, such as Template Load: self.root is "D:\website\app_1\template"
but some common template file is not include in this path. so When I use the {% extends%} like follow:
{% extends "../../template/common/layout.html" %}
I found if the "../../template/common/layout.html" has "{% include logo.html%}", the logo.html cannot be found in the current folder.

Website tree like that:
|- template
| - common | |- layout.html #{%include logo.html%} |- logo.html #ERROR, cannot found this path
- app - template
`- main.html -- #{%extend ../../template/common/layout.html%}

I found there is a bug in class Loader(BaseLoader) resolve_path method:

def resolve_path(self, name, parent_path=None):
    if parent_path and not parent_path.startswith("<") and \
       not parent_path.startswith("/") and \
       not name.startswith("/"):
        current_path = os.path.join(self.root, parent_path)
        file_dir = os.path.dirname(os.path.abspath(current_path))
        relative_path = os.path.abspath(os.path.join(file_dir, name))
        if relative_path.startswith(self.root):
            name = relative_path[len(self.root) + 1:]
        #BUG: should add else here
        else:
            name = relative_path
    return name

bdarnell:
Hmm, this code is strange - it doesn't really make sense to call abspath and put the result in a variable called "relative_path", but it's been there since the beginning. It looks like there are other issues too - the startswith("/") calls should probably be os.path.isabs() to work on windows. If we make that change, your proposed fix no longer works, since layout.html's "name" will be absolute and it will no longer try to load logo.html as a relative path.

It doesn't feel right to use a template loader to access files outside of its root in the first place. I think it would be better if your loader's root was the common ancestor of all the templates you need. Or maybe we need a way to chain multiple loaders together. I'll have to think more about this code and what it's trying to do.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions