Opened 17 years ago

Closed 15 years ago

#883 defect closed fixed (fixed)

[PATCH] [TEST] twisted.web.client thinks ConnectionLost pages are successful

Reported by: Tv Owned by:
Priority: high Milestone:
Component: web Keywords:
Cc: Tv, dp, hypatia Branch:
Author:

Description


Attachments (4)

foo5.py (634 bytes) - added by Tv 17 years ago.
web-client-connectionLost-means-success.diff (811 bytes) - added by Tv 17 years ago.
abortedConnectionTest.diff (2.5 KB) - added by hypatia 17 years ago.
updated-webclient-tests.patch (3.0 KB) - added by Jean-Paul Calderone 16 years ago.
Updated test case patch and a couple more tests

Download all attachments as: .zip

Change History (11)

comment:1 Changed 17 years ago by Tv

When a TCP connection for twisted.web.client.HTTPPageGetter aborts
(ConnectionLost) with no data, it claims a successful retrieval of a zero-length
page.

Here's a demonstration app and a quick patch. I have no clue whether the patch
breaks something unrelated.

See also #882, which is how this got triggered in the first place.

Changed 17 years ago by Tv

Attachment: foo5.py added

comment:2 Changed 17 years ago by hypatia

Attaching diff against twisted/web/test/test_webclient.py -- a test case for
this bug.

Changed 17 years ago by hypatia

Attachment: abortedConnectionTest.diff added

comment:3 Changed 17 years ago by hypatia

Missed the attachment...

Note that it looks like the fix is incorrect: in this particular case, even
though the TCP connection was closed cleanly (reason(ConnectionDone) not
reason(ConnectionLost)) it still seems sensible to fire the errback chain rather
than the callback chain because an HTTP session didn't take place.

Changed 16 years ago by Jean-Paul Calderone

Updated test case patch and a couple more tests

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

Owner: changed from dp to Tv

The attached tests still don't address the original issue, but I think the original issue is less important. Don't use HTTPPageGetter directly: it doesn't work by itself.

comment:5 Changed 15 years ago by Glyph

Owner: changed from Tv to Glyph
Status: newassigned

comment:6 Changed 15 years ago by Glyph

Resolution: fixed
Status: assignedclosed

So I applied the tests in trunk and fixed some test bugs, and they passed!

and then I looked at the existing test cases; apparently this bug has been fixed for a while, since there are other tests which test the same stuff.

comment:7 Changed 11 years ago by <automation>

Owner: Glyph deleted
Note: See TracTickets for help on using tickets.