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

Allow separate storage of offline manifest #1112

Closed
th3hamm0r opened this issue May 18, 2022 · 6 comments
Closed

Allow separate storage of offline manifest #1112

th3hamm0r opened this issue May 18, 2022 · 6 comments

Comments

@th3hamm0r
Copy link
Contributor

By default the manifest file is stored in the same storage as the compressed files. Usually this is static/CACHE/manifest.json.
The problem with that is, that this gives an easy access to all used files, which can be an issue if some of them are used in protected areas.
Currently, you cannot change that by configuration. Using a relative path for COMPRESS_OFFLINE_MANIFEST, e.g. ../../../manifest.json, won't work, because that will fail due to django's protection against path traversal.
In our projects we're using a subclass of CompressorFileStorage which bypasses those checks only for the manifest file, but that's not a clean solution.

Solution:
Similar to how django has fixed that same issue for the manifest files of any ManifestStaticFilesStorage with django/django#12187 in 4.0, we should also allow to configure a separate file storage for the manifest file.
With that it would be possible to store the file in a private directory.

@carltongibson
Copy link
Contributor

The linked PR in Django was relatively painless... It's not up to me but, I'd be inclined to think it reasonable enough here (yes).

Would you be prepared to make a PR?

@diox
Copy link
Member

diox commented May 18, 2022

I'll be happy to review and merge a PR with tests & documentation implementing this.

@th3hamm0r
Copy link
Contributor Author

I haven't contributed to django-compressor yet, but I could try a PR, just I cannot promise that for the near future. So if someone else wants to implement that, I'm ok with it ;)

@carltongibson
Copy link
Contributor

@th3hamm0r — No problem. No rush 😃 — The Django PR you linked should be a nice guide.

Welcome aboard! ⛵

th3hamm0r added a commit to th3hamm0r/django-compressor that referenced this issue May 24, 2022
@th3hamm0r
Copy link
Contributor Author

Thanks for those kind responses! Unexpectedly, I've found some time for #1113.

The Django PR you linked should be a nice guide.

Since the whole manifest code was already separated from the CompressorFileStorage, my approach is a bit different to the django PR.

th3hamm0r added a commit to th3hamm0r/django-compressor that referenced this issue May 24, 2022
@diox diox closed this as completed in d70f1d6 May 27, 2022
@diox
Copy link
Member

diox commented May 27, 2022

I'll release a new version with that change in the coming days/weeks, in the meantime you can use the development branch to get it.

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