Dynamic Image Serve View is failing

[KalobTaulien]

Issue Summary

When using the Dynamic image serve view, from the documentation (https://docs.wagtail.org/en/stable/advanced_topics/images/image_serve_view.html) the URL's are generated, but they cannot be accessed from a GET request.

Steps to Reproduce

1. Follow setup instructions here https://docs.wagtail.org/en/stable/advanced_topics/images/image_serve_view.html
2. Add an image = models.ForeignKey( get_image_model(), ...) field to the HomePage class. Make migrations and apply them.
3. Use the dynamic serve view in the template. Example code below:

{% extends "base.html" %}
{% load wagtailimages_tags %}

{% block content %}
    {% image_url page.image "fill-150x150" as this_works %}
    {{ this_works }}

    {# This does not work #}
    <img src="{{ this_works }}" alt="">
{% endblock content %}

Other details

Terminal output is:

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/wsgiref/handlers.py", line 138, in run
    self.finish_response()
  File "/Users/kalobtaulien/Sites/tmp/svgtest/.venv/lib/python3.11/site-packages/django/core/servers/basehttp.py", line 173, in finish_response
    super().finish_response()
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/wsgiref/handlers.py", line 183, in finish_response
    for data in self.result:
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/wsgiref/util.py", line 24, in __next__
    data = self.filelike.read(self.blksize)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: read of closed file

What the browser outputs:

• I have confirmed that this issue can be reproduced as described on a fresh Wagtail project: yes

Technical details

Python: 3.11
Wagtail: 6.0.1
Django: 5.0.2

Working on this

Anyone can contribute to this. View our contributing guidelines, add a comment to the issue once you’re ready to start.

[LANCECORREIA]

Hey, I am new to wagtail and I would like to pick this issue. To my understanding this issue is occurring because of removing rendition.file.open("rb") in this commit 8fcf6ec by @RealOrangeOne. So was there a reason for removing it?

[RealOrangeOne]

This does indeed look like a bug. I'd removed it because in testing it hadn't made a difference. It seems in some cases, it does.

FileResponse looks like it assumes it's called with an open file handle, so we probably do need to explicitly open it beforehand. We should definitely find a way to automatically test for this, as it sounds like there might be some coverage issues with that view.

[LANCECORREIA]

@RealOrangeOne thanks for the prompt reply. In addition to adding back rendition.file.open("rb") I would like to create a pr for writing more tests for the view as well. Can you please provide some guidance on the nature of the tests and what I should cover in the tests?

[RealOrangeOne]

Glad to see you're interested in fixing this! I think the main thing will be working out in what case this causes issues currently, and attempting to write a test to address it. I suspect writing a test which causes the issue first is a good way to go, and then add the line back and confirm the test now passes.

The exact cause may depend on the file backend used - it's possible this only occurs with certain backends. @KalobTaulien what storage backend are you using? That might help narrow down a proper fix.

[LANCECORREIA]

Sure I will start working on it and create a pr. Please review it once I finish it and thanks for all the help till now.

[LANCECORREIA]

Hey @RealOrangeOne, even though I can confirm that adding rendition.file.open("rb") back solves this issue, I have been unable to recreate it in the test (i.e. the test does not fail even if the file is closed. I have also looked into the storage backend angle however both the wagtail tests and the bakerydemo site are using FileSystemStorage, however the issue only occurs in the bakerydemo site. Does django treat test client and browser requests/responses differently? Or am I missing something?

[KalobTaulien]

@RealOrangeOne I had tried it with various backends, but this happens even with Wagtail's default setup after running wagtail start mysite and modifying the least amount of local code.

@LANCECORREIA Great job picking up this ticket and running with it, you're doing fantastic!

[Akay7]

Until fix is merged if someone get the same error it could be fixed at your project locally by changing Serve behavior to redirect ie ServeView.as_view(action="redirect")