FTP listings never stop loading on Mac

VERIFIED FIXED in mozilla0.8.1

Status

()

--
blocker
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: sfraser_bugs, Assigned: gordon)

Tracking

({regression})

Trunk
mozilla0.8.1
PowerPC
Mac System 8.5
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: have patch, reviews, readyy for checkin)

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
The PR_Poll fixes on Mac broke FTP, such that directory listings (and maybe 
downloads) appear to never finish. The client never closes the data connection, 
with the result that the control connection waits for ever for the 'Transfer 
complete' notification.

The part of the PR_Poll fix that broke this was the change to GetState():

@@ -1676,6 +1681,8 @@
 {
     OTResult resultOT;
+	size_t   availableData;
     
-	*readReady = fd->secret->md.readReady;
+	resultOT = OTCountDataBytes((EndpointRef)fd->secret->md.osfd, &
availableData);   
+	*readReady = fd->secret->md.readReady && (availableData > 0);
 	*exceptReady = fd->secret->md.exceptReady;
 
This changed the behaviour of PR_Poll/PR_Read such that clients would no longer 
see a 0-byte read to indicate that all the data on a socket had been read (which 
is what FTP relied on).
(Reporter)

Updated

18 years ago
Keywords: regression
Target Milestone: --- → mozilla0.8.1
(Reporter)

Comment 1

18 years ago
Created attachment 27756 [details] [diff] [review]
Fix for the bug, adds more logic to GetState()
(Reporter)

Comment 2

18 years ago
The attached patch fixed the bug with FTP, and does not regress the original 
GetState fix (bug 70408).

Comment 3

18 years ago
This must be fixed for 0.8.1
Severity: normal → blocker

Comment 4

18 years ago
gordon, can you review please?
(Assignee)

Comment 5

18 years ago
Review it?  Simon and I wrote it.  Can you try the patch and verify it works for 
you?  I'll check it in this afternoon as soon as I get the necessary approvals.
Status: NEW → ASSIGNED

Comment 6

18 years ago
It would be better if you didn't use the old-style cast.  Since it's a type with
a simple name, you can use a construction-style cast.  Other than that, sr=scc.

Updated

18 years ago
Whiteboard: have patch, reviews, readyy for checkin
Assuming this gets properly tested and it's deemed low risk we're approving this
for 0.8.1.
(Assignee)

Comment 8

18 years ago
Simon and I tested it, but I was hoping DougT was going to try some of his test 
cases on it as well.  Doug?
(Assignee)

Comment 9

18 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 10

18 years ago
verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.