Opened 22 months ago

Last modified 19 months ago

#6168 enhancement new

Enforce type (bytes) of header name and values in twisted.web.http_headers

Reported by: exarkun Owned by: borko
Priority: normal Milestone:
Component: web Keywords:
Cc: jknight Branch:
Author: Launchpad Bug:

Description

The names of items in the HTTP header are bytes. The values are lists of bytes. Python makes it particularly easy to mix up bytes and text (str). To help developers avoid making this mistake, Headers should apply type checks where applications are supplying values for any of these things.

There's already a type check for values being a list of something.

Attachments (2)

6168.diff (1.5 KB) - added by borko 20 months ago.
6168_v2.diff (4.7 KB) - added by borko 20 months ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 22 months ago by DefaultCC Plugin

  • Cc jknight added

Changed 20 months ago by borko

comment:2 Changed 20 months ago by borko

  • Keywords review added
  • Owner set to borko

Changed 20 months ago by borko

comment:3 Changed 20 months ago by borko

  • Owner borko deleted

comment:4 Changed 20 months ago by exarkun

#6186 was a duplicate of this.

comment:5 Changed 19 months ago by ralphm

  • Keywords review removed
  • Owner set to borko

Thanks for you patch!

  1. Given some recent incompatibility issues with stricter type checking, as in #6245 and #6205, it seems to me we should instead first issue deprecation warnings.
  2. This patch even seems to contradict what was done to resolve #6205 as the added deprecation warning is tested to no longer being issued.
  3. The existing test test_nonByteHeaderValue is changed to no longer test what it was supposed to: the ability to pass non-bytes and have them cast transparently. If we really want to disallow this in the future, this test should instead be changed to check for warnings/exceptions.
  4. All the whitespace changes that add spaces around the = sign for keyword arguments or default parameter values should be undone, per our coding standard that depends on 8.
  5. The docstring of the new test test_nonByteHeaderError doesn't actually represent what the test is doing.
  6. I'm not sure why test_nonByteHeaderError has explicit casts to int for integer literals.
  7. Why use intlist.__str__() instead of just str(intlist)?
  8. Tests for setRawHeaders should really go into twisted.web.test.test_http_headers.
  9. Our coding standard requires two blank lines between methods.
  10. I do like that if we would start issuing warnings, they are done from setRawHeaders instead of in Request.write as with the exceptions in this patch.
Note: See TracTickets for help on using tickets.