Ticket #6168 enhancement new

Opened 7 months ago

Last modified 4 months ago

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

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

Change History

1

Changed 7 months ago by DefaultCC Plugin

  • cc jknight added

Changed 5 months ago by borko

2

Changed 5 months ago by borko

  • keywords review added
  • owner set to borko

Changed 5 months ago by borko

3

Changed 5 months ago by borko

  • owner borko deleted

4

Changed 5 months ago by exarkun

#6186 was a duplicate of this.

5

Changed 4 months ago by ralphm

  • owner set to borko
  • keywords review removed

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  pep: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.