Opened 5 years ago

Closed 19 months ago

#6168 enhancement closed wontfix (wontfix)

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

Reported by: Jean-Paul Calderone Owned by: borko
Priority: normal Milestone:
Component: web Keywords:
Cc: jknight Branch:
Author:

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 5 years ago.
6168_v2.diff (4.7 KB) - added by borko 5 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 5 years ago by DefaultCC Plugin

Cc: jknight added

Changed 5 years ago by borko

Attachment: 6168.diff added

comment:2 Changed 5 years ago by borko

Keywords: review added
Owner: set to borko

Changed 5 years ago by borko

Attachment: 6168_v2.diff added

comment:3 Changed 5 years ago by borko

Owner: borko deleted

comment:4 Changed 5 years ago by Jean-Paul Calderone

#6186 was a duplicate of this.

comment:5 Changed 4 years 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.

comment:6 Changed 19 months ago by hawkowl

Resolution: wontfix
Status: newclosed

As mentioned in #6081, #8129 is the preferred way forward for this.

Closing as wontfix.

Note: See TracTickets for help on using tickets.