Opened 13 years ago

Closed 13 years ago

#15668 closed New feature (fixed)

Generic views should provide a HEAD implementation

Reported by: Jamie Matthews Owned by: nobody
Component: Generic views Version: 1.3
Severity: Normal Keywords:
Cc: Tom Christie Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, HEAD requests are responded to with 405 METHOD NOT ALLOWED and an Allow: get header.

$ curl -I localhost:8000
HTTP/1.0 405 METHOD NOT ALLOWED
Date: Wed, 23 Mar 2011 13:04:05 GMT
Server: WSGIServer/0.1 Python/2.6.1
Content-Type: text/html; charset=utf-8
Allow: get

Generic views (or perhaps even the base View class?) should provide a head() implementation that should probably just call the get method (if present) and then throw away the content.

Attachments (2)

15668.patch (3.0 KB ) - added by Jamie Matthews 13 years ago.
15668.2.patch (1.8 KB ) - added by Jamie Matthews 13 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 by Jamie Matthews, 13 years ago

Version: 1.21.3-rc1

comment:2 by Aymeric Augustin, 13 years ago

Django already strips the body of HEAD requests automatically, so head = get is enough.

This is tangentially related to #15637.

by Jamie Matthews, 13 years ago

Attachment: 15668.patch added

comment:3 by Jamie Matthews, 13 years ago

Has patch: set
Patch needs improvement: set

I have added an initial patch, which probably needs improvement.

  1. Is the as_view method the correct place to add in the head = get patch?
  1. I had to make some changes to the tests to make this work. All the other tests in the ViewTest class use a RequestFactory, but I had to use the TestClient to make sure the content of the response was stripped correctly. This begs the question: should the content removal be done in the View class itself to prevent inconsistencies? Something like this maybe:
def head_if_get(self, request, *args, **kwargs):
    # This method adds support for HEAD requests. If
    # a View subclass provides a get method, this method
    # is automatically aliased to head by as_view above.
    def remove_content(response):
        response.content = ''
    response = self.get(request, *args, **kwargs)
    if hasattr(response, 'add_post_render_callback'):
        response.add_post_render_callback(remove_content)
    else:
        remove_content(response)
    return response

Then

if hasattr(self, 'get') and not hasattr(self, 'head'):
    self.head = self.head_if_get

comment:4 by Jamie Matthews, 13 years ago

Version: 1.3-rc11.3

comment:5 by Graham Dumpleton, 13 years ago

Python web applications should never themselves discard the response content for HEAD requests. They should always return the request content and allow the underlying WSGI server or hosting web server to discard the request content. If you don't do that and discard request content anyway, you violate the principal that GET and HEAD should return the same headers, because by removing the response content you prevent a WSGI middleware or web server output filter from processing response content and modifying the response headers. Go read 'http://blog.dscpl.com.au/2009/10/wsgi-issues-with-http-head-requests.html'.

So, if Django is already itself dropping response content for HEAD requests, it technically is not a well behaved WSGI application.

by Jamie Matthews, 13 years ago

Attachment: 15668.2.patch added

comment:6 by Jamie Matthews, 13 years ago

Interesting. I had no idea this was going to be a can of worms..

I've attached a new patch which maintains the same simple head = get behaviour in the View class. I've simplified the tests to punt on checking that response.content = '' (just check that response.status_code == 200).

I can't find any tests for the current handling of HEAD requests, so I'm not sure how much this behaviour has been thought through. Maybe this issue should be raised separately.

comment:7 by Russell Keith-Magee, 13 years ago

Triage Stage: UnreviewedAccepted

Handling of HEAD requests is something that has been there from the beginning, so it's entirely likely that it's untested. As for the WSGI compliance -- it's never come up, so it. Django does drop the content for HEAD requests, so if that is the wrong thing to be doing (and based on Graham's comments, it looks like it might be in the WSGI context) there is a need for another ticket.

By way of explanation -- In this case, I suspect Django's behavior predates our support for WSGI, and for mod_python, truncation would be (AFAICT) the right behavior.

comment:8 by Graham Dumpleton, 13 years ago

Technically, even in mod_python it was the wrong thing to be doing because Apache can supply output filters that are dependent on response content. For example, DEFLATE and CONTENT_LENGTH output filters.

comment:9 by Tom Christie, 13 years ago

Cc: Tom Christie added

comment:10 by Luke Plant, 13 years ago

Type: New feature

comment:11 by Luke Plant, 13 years ago

Severity: Normal

comment:12 by Luke Plant, 13 years ago

Easy pickings: unset
Patch needs improvement: unset
Resolution: fixed
Status: newclosed

Fixed in [16095], thanks. Fat fingers here got the ticket number wrong in the commit message.

BTW, in the future, if you remove the 'needs improvement' flag when you think you've fixed any issues in the patch, it will help move the ticket along.

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