Opened 4 years ago

Closed 4 years ago

#16581 closed Cleanup/optimization (fixed)

Documentation of HttpRequest.META could note possibly surprising type values

Reported by: RoySmith Owned by: nobody
Component: Documentation Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

HttpRequest.METASERVER_PORT?, returns a string (i.e. "80"). It really should be an integer.

Some argument could be made that it really *should* be a string (see http://tinyurl.com/3fpb6px). If it is decided that string is correct, then at least it should be clearly documented. I think most people would expect a network port to be an integer.

Attachments (1)

16581.diff (1.7 KB) - added by BernhardEssl 4 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 4 years ago by kmtracey

  • Component changed from HTTP handling to Documentation
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from HttpRequest.META['SERVER_PORT'] should be an integer to Documentation of HttpRequest.META could note possibly surprising type values
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/optimization

SERVER_PORT is coming from the underlying (or copied in, for older Django) Python wsgiref implementation (see http://hg.python.org/cpython/file/532cad687332/Lib/wsgiref/simple_server.py#l56). Note CONTENT_LENGTH is also a string value. I don't believe Django should change either of these, but if someone wants to provide a doc patch that notes that these values in Request.META are strings I suppose that might avoid some confusion for some people.

comment:2 Changed 4 years ago by RoySmith

Since this appears to be an intentional decision, and changing it would break backwards compatibility, I agree that documenting this (and CONTENT_LENGTH, and any others) would be a sufficient fix.

Changed 4 years ago by BernhardEssl

comment:3 Changed 4 years ago by BernhardEssl

  • Has patch set

I added a patch with warning signs :).
I've also changed the description of the other keys a bit.

comment:4 Changed 4 years ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

The patch looks good to me.

comment:5 Changed 4 years ago by mtredinnick

Out of deference to people who have English as a second (or third or seventeenth) language, I'm going to leave most of these changes as they currently are. My understanding (from doing a internationalisation support for most of a decade) is that possessive form constructions are harder to unambiguously understand than the slightly more passive phrasing we have here. It's a bit of a judgement call, I'll admit.

Will commit the additions for LENGTH, TYPE, etc, though.

comment:6 Changed 4 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

In [16644]:

Improved documentation around HTTP server meta variables.

Fixed #16581, with thanks to Bernhard Essl.

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