tornado icon indicating copy to clipboard operation
tornado copied to clipboard

Template loader problems with absolute paths

Open bdarnell opened this issue 12 years ago • 9 comments

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.

bdarnell avatar Apr 14 '13 18:04 bdarnell

Related feature request: it should be possible to specify a sequence of template search paths or loaders.

bdarnell avatar Apr 14 '13 18:04 bdarnell

It should also be possible to use filenames relative to the template path in extends and includes directives, not just relative to the containing file.

bdarnell avatar Oct 11 '14 15:10 bdarnell

Having a feature that sets directories for templates to be searched would be very useful. I put my route handlers in a sub-directory. Some of those routes will render a template, which has a base template that is extended.

/
/routes/ {api,web,resources}
/templates/ {index, base}

This raises the FileNotFoundError, because my index looks for base in 'routes/templates/' folder. The error didn't occur when my routes were in the parent directory, I'll probably have to move them back.

NucleaPeon avatar Jul 22 '21 20:07 NucleaPeon

My firenado project use a component structure where each component has it's template root.

If you render a template it will get the file located at the template directory in the component root path. You can specify a template from another component using the notation "<template_name>:<template_filename_or_relative_path>" when I see the ":" in the template reference we never have a problem resolving the template.

The issue I'm having that relates to the what you're describing here is when I'm using includes and mixing templates from different templates. Some how I loose reference from where I'm in.

I'll be taking a look closely at this issue. Maybe this issue could get some help on my side. Right now I'm forcing myself to always use the component reference to avoid issues including templates that has included things inside.

Here is my template implementation: https://github.com/candango/firenado/blob/develop/firenado/tornadoweb.py#L430-L502

piraz avatar Jul 24 '21 17:07 piraz

@NucleaPeon It sounds like you're just using the default template_path (which defaults to the directory where your handlers live). You should be able to set template_path (as a keyword argument to the Application constructor) to do what you want.

@piraz Hmm, a component namespace is an interesting idea. This sounds like the template loader needs to be able to override the isabs logic, instead of just fixing it to work for both posix and windows filesystems.

bdarnell avatar Jul 25 '21 18:07 bdarnell

The namespace idea is really nice (although this convention ought to be called something other than "namespace" because of template namespace). It will also allow for distributing packages with their own handlers and templates which can be extended or included by others.

I'm thinking something like:

  1. A new app setting called template_dirs which will be a dict mapping a namespace to a directory. Example:
    { 'namespace1': 'path/to/dir1/', ... }
    
  2. A new method on RequestHandler called get_template_dirs(template_dirs: dict) which can replace or extend the template_dirs app setting.
  3. Modification to RequestHandler.create_template_loader to accept a new arg called template_dirs.
  4. Modification to template.Loader to accept a new arg called template_dirs.

So:

  1. If the template is referenced something like this: self.render('namespace1:index.html') or {% include 'namspace1:index.html' %}, the loader will look for the file in the dir of that namespace.
  2. Otherwise, it will just work as it always has for backwards compatibility.

I can send a pull request if this approach sounds fine.

bhch avatar Nov 05 '21 17:11 bhch

@bhch just take some time testing some scenarios like:

  • namespace2:template1.html: extends namespace1:base.html (full namespace reference, different namespaces)
  • namespace2:template1.html: includes namespace2:template2.html (no namespace reference, same namespace)
  • namespace2:template2.html: includes namespace1:header.html (full namespace reference, different namespaces)
  • namespace1:header.html: includes namespace1:menu.html(no namespace reference, same namespace)

In my case, the way I've implemented, it seems like at the second step, the template loader would try to find template2.html in namespace1 instead of namespace2. I still need to figure what I'm missing here. As a workaround, I'm using full namespace references all the time.

There are mainly two ways to handle this situation:

  • figure out how to pin the current namespace while using includes, extends, etc inside the current template.

This way is more flexible, people can omit the namespace reference if the other template located in the same directory than the current, but will more complexity to the implementation. I thinking that could affect static templates and other in-house template handling solutions.

As soon people start to include and extend templates in different namespaces things can get confusing really fast.

  • force (by convention) that is mandatory the full namespace reference as soon template_dirs is provided to the application

This seems to be more straight-forward but will be very rigid and triggered by and in the whole application.

piraz avatar Nov 06 '21 01:11 piraz

@piraz I did some tests based on the cases you presented. This is how the resolver will work:

  1. If there's a namespace present, find the template in the directory of the namespace i.e. namespace:template1.html will be evaluated as /path/to/namespace1/dir/template1.html.
  2. If there's no namespace reference, find the template relative to the current file.

figure out how to pin the current namespace while using includes, extends, etc inside the current template.

This is already taken care of in the current implementation. The Loader.resolve_path accepts an argument called parent_path which will be the path of the parent template. So, if a template namespace2:template1.html includes template2.html (without namespace) the parent_path will be of namespace2:template1.html and the loader will look for the file in namespace2 dir. I think that's what you mean, right?

bhch avatar Nov 06 '21 09:11 bhch

That's what I meant. Just keep an eye on recursive scenarios @bhch.

piraz avatar Nov 08 '21 01:11 piraz