| msg292263 - (view) |
Author: Mike (jugglinmike) * |
Date: 2017-04-25 14:44 |
| 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] https://github.com/w3c/web-platform-tests/issues/5668 |
|
|
| msg292264 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2017-04-25 14:58 |
| 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 :) |
|
|
| msg292266 - (view) |
Author: Mike (jugglinmike) * |
Date: 2017-04-25 15:55 |
| That would certainly satisfy me! |
|
|
| msg292429 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2017-04-27 12:15 |
| 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. |
|
|
| msg292432 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2017-04-27 12:27 |
| 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). |
|
|
| msg292667 - (view) |
Author: Mike (jugglinmike) * |
Date: 2017-05-01 15:39 |
| 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? |
|
|
| msg294364 - (view) |
Author: Mike (jugglinmike) * |
Date: 2017-05-24 15:40 |
| 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? |
|
|
| msg294373 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2017-05-24 18:25 |
| New changeset a083c8ee6cb89ca573fe10c4c1922e2fa69ceaa3 by R. David Murray (jugglinmike) in branch 'master': bpo-30160: Clarify intended usage of wfile (gh-1300) https://github.com/python/cpython/commit/a083c8ee6cb89ca573fe10c4c1922e2fa69ceaa3 |
|
|
| msg294374 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2017-05-24 18:28 |
| 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. |
|
|
| msg294392 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2017-05-24 21:11 |
| New changeset a2a9984a278d2ee945e91ce13e000e571794fc24 by Zachary Ware in branch '3.6': [3.6] bpo-30160: Clarify intended usage of wfile (gh-1300) (GH-1793) https://github.com/python/cpython/commit/a2a9984a278d2ee945e91ce13e000e571794fc24 |
|
|
| msg294393 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2017-05-24 21:11 |
| New changeset aa27f0e94f4d9c5ef589984dc2b32d856e8e038f by Zachary Ware in branch '3.5': [3.5] bpo-30160: Clarify intended usage of wfile (gh-1300) (GH-1792) https://github.com/python/cpython/commit/aa27f0e94f4d9c5ef589984dc2b32d856e8e038f |
|
|
| msg294394 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2017-05-24 21:11 |
| Backported. Thanks for the patch, Mike! |
|
|
| msg294395 - (view) |
Author: Mike (jugglinmike) * |
Date: 2017-05-24 21:14 |
| My pleasure. And thank you for backporting on my behalf :) |
|
|