BaseHTTPRequestHandler.wfile: supported usage unclear

[jugglinmike]

BPO	30160
Nosy	@bitdancer, @vadmium, @zware, @jugglinmike
PRs	bpo-30160: Clarify intended usage of request handler stream #1300 [3.5] bpo-30160: Clarify intended usage of wfile (gh-1300) #1792 [3.6] bpo-30160: Clarify intended usage of wfile (gh-1300) #1793

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-05-24.21:11:59.242>
created_at = <Date 2017-04-25.14:44:44.921>
labels = ['3.7', 'docs']
title = 'BaseHTTPRequestHandler.wfile: supported usage unclear'
updated_at = <Date 2017-05-24.21:14:17.610>
user = 'https://github.com/jugglinmike'

bugs.python.org fields:

activity = <Date 2017-05-24.21:14:17.610>
actor = 'jugglinmike'
assignee = 'docs@python'
closed = True
closed_date = <Date 2017-05-24.21:11:59.242>
closer = 'zach.ware'
components = ['Documentation']
creation = <Date 2017-04-25.14:44:44.921>
creator = 'jugglinmike'
dependencies = []
files = []
hgrepos = []
issue_num = 30160
keywords = []
message_count = 13.0
messages = ['292263', '292264', '292266', '292429', '292432', '292667', '294364', '294373', '294374', '294392', '294393', '294394', '294395']
nosy_count = 5.0
nosy_names = ['r.david.murray', 'docs@python', 'martin.panter', 'zach.ware', 'jugglinmike']
pr_nums = ['1300', '1792', '1793']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue30160'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[jugglinmike]

The documentation for BaseHTTPRequestHandler explicitly prohibits protocol
violations when writing to the wfile stream:

BaseHTTPRequestHandler has the following instance variables:

[...]

wfile

Contains the output stream for writing a response back to the client.
Proper adherence to the HTTP protocol must be used when writing to this
stream.

I am interested in testing web browser behavior in response to protocol
violations, and my initial interpretation of this text (and the term "must" in
particular) is that such conditions are not guaranteed to achievable with this
module. However, my colleague believes the text is simply intended to
communicate that there is no safety mechanism in place, and that protocol
violations will not be corrected. [1]

Local testing and a quick reading of the source tends to confirm the latter
interpretation, but this may simply be coincidental and not necessarily stable
behavior.

If it is in fact stable, then I would like to request a modification to the
documentation. Changing the word "must" to "should" would help, although it
might be better to be more explicit--something like, "Bytes are transmitted
'as-is'; HTTP protocol violations will not be corrected."

Thanks!

[1] web-platform-tests/wpt#5668

[bitdancer]

I would like to say that the latter interpretation is "obviously" correct, except that it clearly wasn't obvious to you. The fact that it "contains the output stream" I would have thought was pretty clear: if you are writing to it, you are responsible for anything you do to that output stream. The "must" refers to the same the the RFC "must" refers to: in order to be RFC compliant, you must conform to the RFC.

Perhaps we could clarify that sentence by saying: "Proper adherence to the HTTP protocol must be used when writing to this stream in order to achieve successful interoperation with http clients." I think that would make it clear that if you don't, you'll be testing the client's response to protocol violations :)

[jugglinmike]

That would certainly satisfy me!

[vadmium]

The “Proper adherence” sentence has always bothered me. Why does “wfile” have to adhere, but not other other APIs (rfile, send_header, etc)? I wonder if the sentence is useful at all. (Of course you have to use HTTP to operate with HTTP clients.)

Perhaps it was intended to say that socket-level HTTP is written to wfile, that it is up to the caller to ensure the encoding, content length, etc is consistent with the HTTP header, and/or the caller has to supply the header (either direct through wfile or via send_header etc). A plausable alternative would be a higher-level file object like the request body in “http.client”, where encoding and content length is handled by the library.

[bitdancer]

My impression is that the sentence is there because you are acting directly on the raw byte stream, which you are not in the other cases. (Well, rfile, but that's reading, so what you do there doesn't affect the wire protocol you send).

[jugglinmike]

My CLA signature has been verified, but based on the most recent comments, I'm not sure if something needs to change in this patch. Is there anything I can doto help this land?

[jugglinmike]

It's been about a month since I heard back, so I thought I'd comment here just in case this slipped of anyone's radar. Is there anything I can doto help this land?

[bitdancer]

New changeset a083c8e by R. David Murray (jugglinmike) in branch 'master':
bpo-30160: Clarify intended usage of wfile (gh-1300)
a083c8e

[bitdancer]

Pinging the issue is exactly what you needed to do :) As I said on the PR, I'm not set up to do backports yet so hopefully you or someone else will do those.

[zware]

New changeset a2a9984 by Zachary Ware in branch '3.6':
[3.6] bpo-30160: Clarify intended usage of wfile (gh-1300) (GH-1793)
a2a9984

[zware]

New changeset aa27f0e by Zachary Ware in branch '3.5':
[3.5] bpo-30160: Clarify intended usage of wfile (gh-1300) (GH-1792)
aa27f0e

[zware]

Backported. Thanks for the patch, Mike!

[jugglinmike]

My pleasure. And thank you for backporting on my behalf :)