Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leading slash in template names #1981

Open
prg opened this issue Sep 22, 2022 · 4 comments
Open

Leading slash in template names #1981

prg opened this issue Sep 22, 2022 · 4 comments

Comments

@prg
Copy link

prg commented Sep 22, 2022

  • Mojolicious version: 9.22
  • Perl version: 5.32.1
  • Operating system: Ubuntu 20.04

Steps to reproduce the behavior

Having a leading slash in template names (e.g. $c->render(template => '/my/template')) can lead to unexpected behavior.

Mojo Lite with templates in the DATA section flat out won't find them:

use Mojolicious::Lite -signatures;

get '/a' => sub ($c) {
    $c->render(template => '/a');
};

get '/b' => sub ($c) {
    $c->render(template => 'a');
};

app->start;

__DATA__

@@ a.html.ep
A

curl-ing /a:

[2022-09-22 09:54:09.80607] [3181531] [trace] [4Rg79I7N6XFk] GET "/a"
[2022-09-22 09:54:09.80668] [3181531] [trace] [4Rg79I7N6XFk] Routing to a callback
[2022-09-22 09:54:09.80745] [3181531] [trace] [4Rg79I7N6XFk] Template "/a.html.ep" not found
[2022-09-22 09:54:09.80969] [3181531] [error] [4Rg79I7N6XFk] Could not render a response

OTOH:

$ curl localhost:3000/b
A

While this is only a minor issue, it gets weird when using a full-blown Mojolicious app with template variants:

package Variant;
use Mojo::Base 'Mojolicious', -signatures;

sub startup ($self) {
    $self->routes->get('/a' => sub ($c) {
        $c->render(template => '/a');
    });

    $self->routes->get('/b' => sub ($c) {
        $c->render(template => '/a', variant => 'b');
    });

    $self->routes->get('/c' => sub ($c) {
        $c->render(template => 'a', variant => 'b');
    });
}

1;

Templates:

# a.html.ep
A

# a.html+b.ep
B

Instead of bailing on templates with leading slashes (like Mojo Lite), Mojolicious finds them just fine:

$ curl localhost:3000/a
A

But things break when variants come into play:

$ curl localhost:3000/b
A
$ curl localhost:3000/c
B

Running this through the debugger, I found that the problem (in the latter case) seems to be here in template_name:

  if (defined(my $variant = $options->{variant})) {
    $variant = "$template+$variant";
    my $handlers = $self->{templates}{$variant} // [];
    $template = $variant if @$handlers && !defined $handler || grep { $_ eq $handler } @$handlers;
  }

$self->{templates} doesn't have leading slashes, so the variant never gets used.

I honestly don't know what the right answer is here, but I guess somewhat more consistent behavior would be good. No idea how we can achieve that without breaking existing code.

@Grinnz
Copy link
Contributor

Grinnz commented Sep 22, 2022

I am so confused. Why are you putting slashes in your template names?

@prg
Copy link
Author

prg commented Sep 22, 2022

I am so confused. Why are you putting slashes in your template names?

Going straight for the existential questions, tbqh I don't know. While looking into this issue I noticed I have a wild mix of both (with and without leading slash) in my current codebase, can't quite remember how it was in previous projects.

But that's kinda my point, right now a leading slash works in most cases. Do we want to "fix" this by disallowing leading slashes? That could lead to major breakage for some people...

@kraih
Copy link
Member

kraih commented Dec 8, 2022

Think i'm ok with not supporting slash chars in template names.

@prg
Copy link
Author

prg commented Dec 9, 2022

Think i'm ok with not supporting slash chars in template names.

Totally fine by me, I just opened this issue because you said something along the lines of "This shouldn't happen".

Maybe we can find a nice spot for a notice or warning in the documentation so this information doesn't get buried in a closed issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants