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

Error when HTTP Accept headers reference template formats that don't exist #2705

Closed
immersegfx opened this issue Oct 24, 2019 · 3 comments
Closed
Assignees
Labels

Comments

@immersegfx
Copy link

I've been seeing errors when users are accessing the site with different types of HTTP Accept headers. I believe it has something to do with the order of each property. This has only presented it's self for modular pages at this time.

For example when someone visits and text/html is listed first the page renders fine.
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng

However if application/json or another is listed first and Exception is thrown
Accept: application/json; text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,/;q=0.8

I used a plugin in chrome to test this out and was able to replicate the issue on getgrav.org. When visiting getgrav.org and setting those headers it throws the server error below.

0 - An exception has been thrown during the rendering of a template ("Template "modular/people-saying.xml.twig" is not defined.").

I was able to circumvent the issue by hooking onto the onTwigPageVariables event and checking to see that the template exists and falling back to html.

public function onTwigPageVariables(Event $event) { 
    $page = $event['page'];
    $grav = Grav::instance();
    if($page->modularTwig()) {
      $extension = $page->templateFormat();
      $extension = $extension ? ".{$extension}.twig" : TEMPLATE_EXT;
      $template = $page->template() . $extension;
      $theme = $grav['locator']->findResource('theme://templates/' . $template);
      $plugin = $grav['locator']->findResource("plugin://{$this->name}templates/" . $template);
      if(!$theme && !$plugin) {
        // Force the template format to HTML.  If that doesn't exist we have bigger problems
        $page->templateFormat('html');
      }
    }
}

It appears to me that when handling the request. Grav loops through the page types and tries to match against the first Accept type it finds, and continues to try and load the template even if it doesn't exist. I've tried to reorder the page types in the configuration settings with no success.

I question if there are any advantages to having variable page types in modular templates? Perhaps the children should inherit the characteristics of the parent?

@mahagr
Copy link
Member

mahagr commented Oct 24, 2019

I think there may be an issue on file format detection; the second entry contains 3 types of html variants:

  • text/html
  • application/xhtml+xml
  • application/xml

First one should match in this case, thus trying to get html template.

@rhukster
Copy link
Member

I need to investigate this. We use the Negotiation library to interpret the Accept: headers as this can actually get quite complicated. There is no 'existence' validation happening however. I will look to see what is the most 'performant' way of handling this.

@rhukster
Copy link
Member

I've fixed this in 1.7 branch ready for 1.7.-rc.1 release which should be this week.

@mahagr mahagr closed this as completed Jan 3, 2020
@mahagr mahagr added bug and removed fixed labels Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants