[Twisted-Python] HTTPClient handling LF only servers
Hi Y'all, Certain servers (notably Hacker News) break HTTPClient because they use LF instead of CRLF to terminate header lines. I've uploaded a patch with tests to ticket 4814 (merged into ticket 2842). I'd appreciate a review and some discussion about integrating it. -J Sent via iPhone Is your e-mail Premiere?
On Tue, Jan 25, 2011 at 11:54 AM, Jason J. W. Williams < jasonjwwilliams@gmail.com> wrote:
Hi Y'all,
Howdy, Jason!
Certain servers (notably Hacker News) break HTTPClient because they use LF instead of CRLF to terminate header lines. I've uploaded a patch with tests to ticket 4814 (merged into ticket 2842).
I'd appreciate a review and some discussion about integrating it.
-J
I'm not really in a position to do a review right now, but since: 1) 4814 is effectively a duplicate of 2842 2) 4814 has a clear problem decription 3) 4814 has a patch 4) 2842 is old, and confusing Can we go ahead and close 2842 in favor of 4814? We have too many old tickets floating around... (To be clear, I'm not asking Jason to close this ticket...I'm asking for the list's opinion) Also 3833 is at least similar, if not another duplicate. It would be nice if it could be worked on and/or closed along with this issue. Kevin Horn
On Tue, Jan 25, 2011 at 2:23 PM, Kevin Horn <kevin.horn@gmail.com> wrote:
On Tue, Jan 25, 2011 at 11:54 AM, Jason J. W. Williams < jasonjwwilliams@gmail.com> wrote:
Hi Y'all,
Howdy, Jason!
Certain servers (notably Hacker News) break HTTPClient because they use LF instead of CRLF to terminate header lines. I've uploaded a patch with tests to ticket 4814 (merged into ticket 2842).
I'd appreciate a review and some discussion about integrating it.
-J
I'm not really in a position to do a review right now, but since:
1) 4814 is effectively a duplicate of 2842 2) 4814 has a clear problem decription 3) 4814 has a patch 4) 2842 is old, and confusing
Can we go ahead and close 2842 in favor of 4814? We have too many old tickets floating around...
(To be clear, I'm not asking Jason to close this ticket...I'm asking for the list's opinion)
I most certainly agree that 4814 is the more relevant, actionable ticket. 2842 is marred in now-useless history and requires reading through 13 comments just to get to the current point of the ticket as the summary and description give no indication of it. 4814 is more like a new building constructed on top of 2842's rubble.
Also 3833 is at least similar, if not another duplicate. It would be nice if it could be worked on and/or closed along with this issue.
Agreed about 3833. Who cares about finding sites that do/don't send LF? I could talk about how naively-implemented the whole HN stack is all day, but it wouldn't get us closer to a resolution for Twisted's web client(s). If the RFC recommends it for "Tolerant Applications", the only decision is whether or not we implement that part of the RFC. If it requires more time to discuss than to actually fix, that means we should probably just fix it and be done with it.
Kevin Horn
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
On 07:23 pm, kevin.horn@gmail.com wrote:
On Tue, Jan 25, 2011 at 11:54 AM, Jason J. W. Williams < jasonjwwilliams@gmail.com> wrote:
Hi Y'all, Howdy, Jason! Certain servers (notably Hacker News) break HTTPClient because they use LF instead of CRLF to terminate header lines. I've uploaded a patch with tests to ticket 4814 (merged into ticket 2842).
I'd appreciate a review and some discussion about integrating it.
-J I'm not really in a position to do a review right now, but since:
1) 4814 is effectively a duplicate of 2842 2) 4814 has a clear problem decription 3) 4814 has a patch 4) 2842 is old, and confusing
Can we go ahead and close 2842 in favor of 4814? We have too many old tickets floating around...
#4814 is already closed. If the #2842 description is confusing, then feel free to update it to be more clear. :) We don't close old tickets as duplicates of new tickets, though. That doesn't make sense, and it's annoying to the original reporter/cc list as well.
(To be clear, I'm not asking Jason to close this ticket...I'm asking for the list's opinion)
Also 3833 is at least similar, if not another duplicate. It would be nice if it could be worked on and/or closed along with this issue.
Similar, certainly. But it's for the other HTTP client implementation. There won't be any code in common in the resolution. I certainly agree that if it's worth fixing for one it's worth fixing for the other, though, and I hope someone will put #3833 up for review soon. :) Jean-Paul
Can we go ahead and close 2842 in favor of 4814? We have too many old tickets floating around...
#4814 is already closed. If the #2842 description is confusing, then feel free to update it to be more clear. :)
We don't close old tickets as duplicates of new tickets, though. That doesn't make sense, and it's annoying to the original reporter/cc list as well.
Honestly, whether 4814 or 2842 is the main ticket for the problem doesn't matter to me. I'd just like to get the fix put in. I believe my patch is attached to 2842 when 4814 got closed as a dup. -J
On Jan 25, 2011, at 3:03 PM, Jason J. W. Williams wrote:
Can we go ahead and close 2842 in favor of 4814? We have too many old tickets floating around...
#4814 is already closed. If the #2842 description is confusing, then feel free to update it to be more clear. :)
We don't close old tickets as duplicates of new tickets, though. That doesn't make sense, and it's annoying to the original reporter/cc list as well.
Honestly, whether 4814 or 2842 is the main ticket for the problem doesn't matter to me. I'd just like to get the fix put in. I believe my patch is attached to 2842 when 4814 got closed as a dup.
Shouldn't 2842 be in review, then? (In case you're not familiar with them: <http://twistedmatrix.com/trac/wiki/ReviewProcess>, <http://twistedmatrix.com/trac/wiki/TwistedDevelopment>.)
On Tue, Jan 25, 2011 at 1:53 PM, <exarkun@twistedmatrix.com> wrote:
On 07:23 pm, kevin.horn@gmail.com wrote:
On Tue, Jan 25, 2011 at 11:54 AM, Jason J. W. Williams < jasonjwwilliams@gmail.com> wrote:
Hi Y'all, Howdy, Jason! Certain servers (notably Hacker News) break HTTPClient because they use LF instead of CRLF to terminate header lines. I've uploaded a patch with tests to ticket 4814 (merged into ticket 2842).
I'd appreciate a review and some discussion about integrating it.
-J I'm not really in a position to do a review right now, but since:
1) 4814 is effectively a duplicate of 2842 2) 4814 has a clear problem decription 3) 4814 has a patch 4) 2842 is old, and confusing
Can we go ahead and close 2842 in favor of 4814? We have too many old tickets floating around...
#4814 is already closed.
Ah. Somehow I thought it was still open.
If the #2842 description is confusing, then feel free to update it to be more clear. :)
Well, I can update the summary, but not the description, which is the really bad part.
We don't close old tickets as duplicates of new tickets, though. That doesn't make sense, and it's annoying to the original reporter/cc list as well.
This certainly makes sense in theory, but when the last comment from the nominal owner of the ticket is "I'm not sure what is supposed to be fixed now.", I'm not sure how helpful it is. Yes, I realize that you had responded to it, and clarified the situation. Just Saying. There's a LOT of mutant tickets out there, that don't mean what they say, or have completely changed in scope. And there are several which have been closed in favor of newer, clearer tickets over the years. I think we need to make the above policy obvious someplace, like in the developer docs. Maybe it's in there, I haven't checked, but I don't recall reading it, and I've read most of the documentation _many_ times. In any case, I don't care _that_ much about _which_ one is closed as long as one is. Which appears to be the case.
(To be clear, I'm not asking Jason to close this ticket...I'm asking for the list's opinion)
Also 3833 is at least similar, if not another duplicate. It would be nice if it could be worked on and/or closed along with this issue.
Similar, certainly. But it's for the other HTTP client implementation. There won't be any code in common in the resolution.
I certainly agree that if it's worth fixing for one it's worth fixing for the other, though, and I hope someone will put #3833 up for review soon. :)
Jean-Paul
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
On Jan 25, 2011, at 6:09 PM, Kevin Horn wrote:
Well, I can update the summary, but not the description, which is the really bad part.
The description _is_ mutable, just only by certain magical people. (For the purpose of spam protection). For now, please feel free to attach a comment to any confusing ticket asking for a description update from someone who can do that; we should do this more often as the foci of various tickets shift. In the near future hopefully a trac admin (maybe me) will have enough time to more effectively curate the group of magical accounts and their attendant powers so that more people (especially documentation wonks like you and Tom) have that ability.
On 06:28 pm, glyph@twistedmatrix.com wrote:
On Jan 25, 2011, at 6:09 PM, Kevin Horn wrote:
Well, I can update the summary, but not the description, which is the really bad part.
The description _is_ mutable, just only by certain magical people. (For the purpose of spam protection).
For now, please feel free to attach a comment to any confusing ticket asking for a description update from someone who can do that; we should do this more often as the foci of various tickets shift. In the near future hopefully a trac admin (maybe me) will have enough time to more effectively curate the group of magical accounts and their attendant powers so that more people (especially documentation wonks like you and Tom) have that ability.
I've added Kevin and Tom to the group who can do this. If someone would like to volunteer for the job of putting trusted people into this group, let me know. Jean-Paul
On Wed, Jan 26, 2011 at 1:39 PM, <exarkun@twistedmatrix.com> wrote:
On 06:28 pm, glyph@twistedmatrix.com wrote:
On Jan 25, 2011, at 6:09 PM, Kevin Horn wrote:
Well, I can update the summary, but not the description, which is the really bad part.
The description _is_ mutable, just only by certain magical people. (For the purpose of spam protection).
For now, please feel free to attach a comment to any confusing ticket asking for a description update from someone who can do that; we should do this more often as the foci of various tickets shift. In the near future hopefully a trac admin (maybe me) will have enough time to more effectively curate the group of magical accounts and their attendant powers so that more people (especially documentation wonks like you and Tom) have that ability.
I've added Kevin and Tom to the group who can do this. If someone would like to volunteer for the job of putting trusted people into this group, let me know.
Thanks, I can now more efficiently exercise my obsessive compulsions!
Jean-Paul
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
participants (5)
-
exarkun＠twistedmatrix.com -
Glyph Lefkowitz -
Jason J. W. Williams -
Kevin Horn -
Tom Davis