Archmonger / ServeStatic

Production-grade static file server for Python web apps.
https://archmonger.github.io/ServeStatic/
MIT License
25 stars 2 forks source link

Use the Django Checks API to test for misconfiguration #64

Open Alexerson opened 2 weeks ago

Alexerson commented 2 weeks ago

Python Version

3.12.7

Django Version

5.1.3

Package Version

2.1.1

Description

With a local setup, my static files are cropped at 8192 characters.

To be honest, I’m not sure what’s happening. I feel this was working fine for a moment, but this morning, all my static assets are cropped after 8192 characters. I feel there is something about chunks not properly reattached somewhere, but I’m a bit puzzled as of what’s up.


ADMIN NOTE: This was discovered to be a middleware misconfiguration issue. Automated checks should be created that tell the user if something is set up wrong.

Alexerson commented 2 weeks ago

Ok, I’m very confused. If I use curl to see what’s up, then the whole file is sent. I’m using with Chrome 130.0, is that a browser issue?

I’ve tried with and without daphne.

Archmonger commented 2 weeks ago

I'll try and replicate this. Are you using ServeStaticASGI or the Django middleware? Is your Django project configured to use ASGI? Additionally, what version of chrome are you using?

Alexerson commented 2 weeks ago

The django middleware. I’m using Chrome 130 on Linux.

Edit: I don’t have much time today, but I’ll also explore more next week. In production, it seems everything works fine. For now I just reverted to whitenoise.

Archmonger commented 1 week ago

Can you try v1.2.0? I have a suspicion it's related to the v2 upgrade.

Alexerson commented 1 week ago

Well, maybe I’m doing something wrong, but I have the same issue.

Archmonger commented 1 week ago

I've been unable to replicate this locally or via new unit tests (#67).

Is there any chance you could create a repo that has a minimal example and share it with me?

Additional information

Django sends out static files in 8192 byte chunks by default. In your scenario, it looks both the server and browser are satisfied with only transmitting the first chunk, which is very strange.

If you use Wireshark to snoop on the HTTP traffic, it would likely show two HTTP requests: The first containing HEAD content then the second containing the first 8192 bye chunk of the body content.

I would be interested to know what your browser's DevTools > Network tab says for the Response Headers -> Content-Length value. It's possible Django is telling the browser the static file is only 8192 bytes long.

Just throwing ideas out there, but this could potentially be a daphne bug. You might want to try running your local app with uvicorn or hypercorn to help narrow things down.

Alexerson commented 2 days ago

I get the same behaviour with uvicorn, or even with a regular manage.py runserver in wsgi (or even just gunicorn with wsgi). So I feel there must be something on my side that is very weird.

I’ll have time this afternoon to explore more so I’ll start from an empty project and will slowly add all my stuff to see when/if it breaks.

Alexerson commented 2 days ago

Ok, I found the issue! In the middleware stack, 'servestatic.middleware.ServeStaticMiddleware' needs to be before 'django.middleware.gzip.GZipMiddleware'!

Archmonger commented 1 day ago

Ahhh this reminds me - I should add some automated checks that throw errors/warnings on Django misconfiguration.

For example, I use the Django Check API on another repo I maintain.

Alexerson commented 1 day ago

Yes, maybe also just mention this in the docs? It was not an issue with Whitenoise for reasons. The django admin app has a good example as well: https://github.com/django/django/blob/857b1048d53ebf5fc5581c110e85c212b81ca83a/django/contrib/admin/checks.py#L141 The only downside is that I don’t think you can register the check without having people add ServeStatic as an installed app (I think, not quite sure, maybe you can register a check without it).

Archmonger commented 1 day ago

It is already mentioned in the docs, but I will convert it from a sentence to a warning admonition.

image

You're right about the Django check requiring an app... Will need to debate whether it's worth further deviating from WhiteNoise to include these checks.

I technically could do a hacky variant of this and introspect settings.py:MIDDLEWARE within each middleware call, but that's not ideal.