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

CSRF errors cause ld_for_site template_tag to fail, causes 500 error #16

Open
dkirkham opened this issue Nov 28, 2022 · 2 comments
Open

Comments

@dkirkham
Copy link

Under some circumstances - such as CSRF errors - django core will attempt to render the 403_csrf.html template, and the context will not include the request key. If the template includes wagtailschemaorg_tags and in particular ld_for_site the template tag will itself cause a KeyError exception. This creates a 500 response and spams the error logs:

...
File "/home/staging/3.8/lib/python3.8/site-packages/django/template/base.py", line 958, in render_annotated
   return self.render(context)
 File "/home/staging/3.8/lib/python3.8/site-packages/django/template/library.py", line 239, in render
   output = self.func(*resolved_args, **resolved_kwargs)
 File "/home/staging/3.8/lib/python3.8/site-packages/wagtailschemaorg/templatetags/wagtailschemaorg_tags.py", line 17, in ld_for_site
   site = Site.find_for_request(context["request"])
 File "/home/staging/3.8/lib/python3.8/site-packages/django/template/context.py", line 83, in __getitem__
   raise KeyError(key)

From what I've read, ideally, template tags facing this sort of problem should fail silently.

As the ld_for_site template tag is often in a base.html file included into most page templates and in this case, the 403_csrf.html template, it is a bit messy to work around this issue in the template system. My workaround, until this package is fixed, is to reimplement ld_for_site in my own code, returning an empty string if the request key is not present.

@dkirkham dkirkham changed the title CSRF errors causes ld_for_site template_tag to fail, causes 500 error CSRF errors cause ld_for_site template_tag to fail, causes 500 error Nov 28, 2022
@seb-b
Copy link
Member

seb-b commented Nov 28, 2022

We could check for request in the template tag, wouldn't hurt

It's tempting to put the tags from this package in a base template but there are templates where that doesn't make sense (404, 500, csrf error etc). Schema tags are usually used for SEO and these pages aren't typically indexed

I'd recommend having a intermediary template that your pages inherit from that includes the schema tags that inherits your base template so the structure looks something like

base.html -> page_base.html (ld tags here) -> page_1.html, page_2.html etc
         \
           -> 404.html, 403_csrf.html etc

@dkirkham
Copy link
Author

Yes, makes sense.

I found another templatetag with the same problem, and I had a deadline, so I've hardcoded the 403_csrf.html for now. I'll apply your solution when I get a chance.

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

2 participants