Skip to content

The static files panel shouldn't choke on unexpected data types #2021

New issue

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

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

Already on GitHub? # to your account

Merged
merged 4 commits into from
Oct 27, 2024

Conversation

matthiask
Copy link
Member

@matthiask matthiask commented Oct 27, 2024

Description

Using Path() instances in static() is unexpected but we shouldn't crash.

While debugging I found that our DebugStaticFilesStorage.url instantiates a StaticFile which in turn calls DebugStaticFilesStorage.url again, so rendering self.panel.content in my test wouldn't end ever. What was tricky about it is that I didn't get an infinite recursion error because we're using a signal. (To be clear: I'm not sure this is what happens, I only think it is.)

I'm confused why this wouldn't have appeared earlier, but I'm happy with the fix.

Fixes #2002

Checklist:

  • I have added the relevant tests for this change.
  • I have added an item to the Pending section of docs/changes.rst.

@matthiask matthiask changed the title StaticFile.__str__ should always return a str, not e.g. a Path() instance The static files panel shouldn't choke on unexpected data types Oct 27, 2024
Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code makes sense. I don't have ideas on why this would break either but I'm happy to see a problem solved

@salty-ivy
Copy link
Member

What was tricky about it is that I didn't get an infinite recursion error because we're using a signal. (To be clear: I'm not sure this is what happens, I only think it is.)

If that were to be the case, wouldn't we will have numerous signals for a single request, if call staticfile.url()

@salty-ivy
Copy link
Member

What was tricky about it is that I didn't get an infinite recursion error because we're using a signal.

Just checked.

we can still hit the maximum recursion depth if we make a call to StaticFile(path).url() within DebugStaticFilesStorage.url()

I think that concludes that signal is not the reason and staticfile.url() is not being called within DebugStaticFilesStorage.url() in any typical case.

@matthiask matthiask merged commit 6d45d1d into django-commons:main Oct 27, 2024
12 checks passed
@matthiask matthiask deleted the 2002 branch October 27, 2024 14:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError __str__ returned non-string (type PosixPath) /debug_toolbar/templates/debug_toolbar/panels/staticfiles.html
3 participants