#30509: Various FileResponse fixes and changes
-------------------------------------+-------------------------------------
Reporter: Piotr | Owner: nobody
Kunicki |
Type: | Status: new
Cleanup/optimization |
Component: HTTP | Version: master
handling |
Severity: Normal | Keywords: FileResponse
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
As I've mentioned in PR 11011 (of ticket #30196) I noticed a couple of
issues with the way FileResponse is created (and other minor things), and
in this ticket I'll try to list and fix them.
For initial clarity, this PR consists of 2 commits. The first is a set of
tests, most of which show invalid behavior in some quite specific cases
based on current code. The second contains my initial proposed fixes.
1. tests:
a. {{{test_file_from_buffer_absolute_name}}}:\\
test ends in an error, because {{{set_headers()}}} attempts to read the
'name' property if the filename (which can be overwritten by the custom
filename) is recognized as an absolute path by os.path.
b. {{{test_file_from_buffer_explicit_type1}}},
{{{test_file_from_buffer_explicit_type2}}}:\\
of these 2, first test shows the way to explicitly set {{{Content-Type}}}
header of the response, while the second shows that in a case that the set
value matches the beginning of the default value ({{{'text/html;
charset=%s' % self.charset}}}), it will consider that value as unset and
proceed to read the content type from the file.
c. {{{test_file_from_named_buffer1}}},
{{{test_file_from_named_buffer2}}}:\\
tests show how you can easily 'fool' FileResponse into thinking it's
dealing with an actual file, resulting in incorrect header values if such
file exists (1) or an error if it doesn't (2).
d. {{{test_file_from_disk_custom_name}}},
{{{test_file_from_disk_as_attachment_custom_name}}}:\\
these are just 2 missing tests to check whether giving a file a custom
name even works.
2. fixes:
a. ignored custom filename before setting {{{Content-Length}}} and
changed the following condition a bit to fix 1.a. and the error case of
1.c.
b. added a {{{self._default_content_type}}} flag to
{{{HttpResponseBase.__init__()}}} to mark that there was no
{{{content_type}}} explicitly set, so it should be figured out from the
file, if possible, to fix 1.b.
c.
* added a way to tell the filelike object's size if it's neither a file
nor has a {{{getbuffer}}} attribute, using {{{.seek()}}} and {{{.tell()}}}
if they're available, and otherwise {{{.read()}}}
* moved {{{os.path.getsize}}} path to be the worst-case-scenario, to
avoid reading the same file from disk again when possible
* that fixes the remaining problem found in 1.c..
d. moved {{{encoding_map}}} to initialize only when it's necessary.
e. changed {{{set_headers()}}} to {{{_set_headers()}}} to mark it as an
internal function.
f. simplified the - IMO - unnecessarily detailed docs of FileResponse to
just tell the user what do the parameters cause, instead of (almost)
explaining what each line of code does - if the user wants to know which
headers get what kind of values under which conditions they can just check
the code, Python is nearly pseudocode anyway.
One thing i'm not sure about is if FileResponse should support reading
filelikes from a different position than the beginning. If so, getting
{{{Content-Length}}} would require a tiny change to accomodate a non-zero
starting point, while otherwise it could use a {{{filelike.seek(0)}}}
somewhere to be sure we're reading it from the very beginning.
--
Ticket URL: <https://code.djangoproject.com/ticket/30509>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/055.f533a9d9066c857bd020bf7ba14d97ed%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
