Opened 3 years ago

Last modified 19 months ago

#24243 new New feature

Allow HashedFilesMixin to handle file name fragments

Reported by: Laszlo Marai Owned by: Laszlo Marai
Component: contrib.staticfiles Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The hashed_name method will raise a ValueError if fed a non-existent file (including a file name fragment) which problematic for several reasons:

  • File name fragments can't be used this way (e.g. I want to do {% url '/static/sounds/some_sound' %} and then append the extension from javascript)
  • The above technique works with cache middle ware not using HashedFilesMixin, so the ValueError comes kind of unexpected (e.g. when only using it on staging/deployment environments and not locally)
  • Actually just below the offending code it explicitly tries to handle fragments, even has a comment about it, but the exception is raised before.

Problematic code starts on line 89.

Change History (17)

comment:1 Changed 3 years ago by Laszlo Marai

Owner: changed from nobody to Laszlo Marai
Status: newassigned

comment:2 Changed 3 years ago by Laszlo Marai

Owner: Laszlo Marai deleted
Status: assignednew

comment:3 Changed 3 years ago by Laszlo Marai

After looking into it, now I see that I misunderstood the word 'fragment' in the comments and it refers to url fragments, not file name fragments (as in partial file names). However, what I have described above is still a valid behaviour and in line with the default django.contrib.staticfiles.storage.StaticFilesStorage behaviour.

comment:4 Changed 3 years ago by Laszlo Marai

Owner: set to Laszlo Marai
Status: newassigned

comment:5 Changed 3 years ago by Tim Graham

Easy pickings: unset

Can you offer a patch for this issue? I'm having trouble understanding exactly what changes you're proposing to make?

comment:6 Changed 3 years ago by Tim Graham

Resolution: needsinfo
Status: assignedclosed

comment:7 Changed 3 years ago by Laszlo Marai

The change I'm suggesting is that resolving a non-existent file name doesn't raise a ValueError when using ManifestStaticFilesStorage (change needed in HashedFilesMixin). Though it was 5 weeks ago, it seems I have done the fix back then.

Do I attach a file containing the suggested fix here or do I create a pull request? (The documentation wasn't 100% clear.)

comment:8 Changed 3 years ago by Tim Graham

Pull requests are preferred. Let me know which documentation you were confused by so we can update it.

comment:9 Changed 3 years ago by Laszlo Marai

Pull request created. Can't remember the documentation that caused the confusion, it was over a month ago. Will let you know if I see it again.

comment:10 Changed 3 years ago by Aymeric Augustin

Isn't this a duplicate of #18958?

comment:11 Changed 3 years ago by Laszlo Marai

Nope, it isn't. What I'm doing is being able to use

{% static 'wherever/some/of/your/files/are/' %}

or e.g. (generated js)

var sound = '{% static 'sounds/mysound' %}';

and then append .mp3 or .ogg depending on the browser capabilities. Both files are present. Actually #18958 would still be an issue for the reporter after accepting my patch.

comment:12 Changed 3 years ago by Tim Graham

Resolution: needsinfo
Status: closednew
Type: UncategorizedNew feature

Reopening for further consideration.

comment:13 Changed 3 years ago by Aymeric Augustin

With the example above, var sound = '{% static 'sounds/mysound' %}'; should be rendered as var sound = '/static/sounds/mysound.e678b76a';. How does Django determine the hash to insert there?

comment:14 Changed 3 years ago by Laszlo Marai

It does not. In this case, cache busting doesn't work.

comment:15 Changed 3 years ago by Tim Graham

Has patch: set
Patch needs improvement: set
Summary: HashedFilesMixin doesn't handle file name fragments (thus the url template tag neither)Allow HashedFilesMixin to handle file name fragments
Triage Stage: UnreviewedAccepted
Version: 1.7master

I'm tentatively accepting this ticket, although I'd like to confirm with @jezdez who authored HashedFilesMixin.

As far as I could tell when using it (I tested with CachedStaticFilesStorage), {% static 'foo/bar' %} (that doesn't exist) will only cause an error when DEBUG=False (without this patch). This doesn't seem like a great behavior (errors go undetected until you're running in production). A better behavior might be log a warning when DEBUG=True. With this patch, references to missing files go undetected, but at least won't get crashes in production for missing files.

comment:16 Changed 19 months ago by Ed Johnson

Hi all,

What happened to this bug? It's killing my deployment to production on Heroku with DEBUG=False. Raising an uncaught ValueError in post_process that kills collectstatic and breaks deployments is a sub optimal design decision.

A single missing static file blocks my entire production deployment for Django 1.7 - 1.9

I should be able to override this behavior with something like an optional --no-strict flag on collectstatic.
The default could be strict (as it is today), and then developers like me could make the choice to get our deployment out to production with a missing file, instead of being completely (and surprisingly) blocked.

What are the steps to re-open this?

comment:17 Changed 19 months ago by Tim Graham

The ticket is open. It's waiting for a patch.

Note: See TracTickets for help on using tickets.
Back to Top