From: Joey Hess <joeyh@joeyh.name>
Date: Tue, 1 Dec 2020 19:39:22 +0000 (-0400)
Subject: avoid hGetMetered 0 closing the handle
X-Git-Tag: archive/raspbian/10.20250416-2+rpi1~1^2~98^2~417
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=92136284b15f40e8e1274c1994f5825e977789f5;p=git-annex.git

avoid hGetMetered 0 closing the handle

This is an edge case, which happened to be triggered by the P2P protocol
seeing DATA 0. When reading 0 bytes, getting an empty string does
not mean the handle has reached EOF.

I verified there was in fact a bug, where get of an empty file followed
by another file would get the empty file and then fail
with "handle is closed". This fixes it.

This commit was sponsored by Boyd Stephen Smith Jr. on Patreon.
---

diff --git a/CHANGELOG b/CHANGELOG
index f2e4b19509..18890f78f0 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,8 @@ git-annex (8.20201128) UNRELEASED; urgency=medium
 
   * Fix hang on shutdown of external special remote using ASYNC protocol
     extension. (Reversion introduced in version 8.20201007.)
+  * Fix bug that made the next download after an empty file from a ssh
+    or tor remote fail.
 
  -- Joey Hess <id@joeyh.name>  Mon, 30 Nov 2020 12:55:49 -0400
 
diff --git a/Utility/Metered.hs b/Utility/Metered.hs
index 888e7e1416..3e2333cc71 100644
--- a/Utility/Metered.hs
+++ b/Utility/Metered.hs
@@ -169,8 +169,9 @@ hGetMetered h wantsize meterupdate = lazyRead zeroBytesProcessed
 		c <- S.hGet h (nextchunksize (fromBytesProcessed sofar))
 		if S.null c
 			then do
-				hClose h
-				return $ L.empty
+				when (wantsize /= Just 0) $
+					hClose h
+				return L.empty
 			else do
 				let !sofar' = addBytesProcessed sofar (S.length c)
 				meterupdate sofar'
diff --git a/doc/bugs/p2p_protocol_misbehavior_when_location_log_out_of_date.mdwn b/doc/bugs/p2p_protocol_misbehavior_when_location_log_out_of_date.mdwn
index 44f0639822..9e967a2000 100644
--- a/doc/bugs/p2p_protocol_misbehavior_when_location_log_out_of_date.mdwn
+++ b/doc/bugs/p2p_protocol_misbehavior_when_location_log_out_of_date.mdwn
@@ -13,3 +13,31 @@ The problem is on the first move, the protocol does not handle a file
 that's not present well, so it's not clear why it failed. And since that
 closes the connection, the next move fails when it should not need to.
 --[[Joey]]
+
+> Protocol dump:
+> 
+> 	P2P > VERSION 1
+> 	P2P < VERSION 1
+>	P2P > GET 0 foo SHA256E-s30--f8f8766a836fb6120abf4d5328ce8761404e437529e997aaa0363bdd4fecd7bb
+> 	P2P < GET 0 foo SHA256E-s30--f8f8766a836fb6120abf4d5328ce8761404e437529e997aaa0363bdd4fecd7bb
+> 	P2P > DATA 0
+> 	P2P > VALID
+>	P2P < DATA 0
+>	P2P > SUCCESS
+>	P2P < SUCCESS
+> 
+> So it's sending an empty object and claiming it's valid when in
+> fact it does not have the object. That was done because the sender
+> does not have any other way in the protocol to indicate that.
+> The receiver is what sends SUCCESS/FAILURE, not the sender.a
+> 
+> Seems like, it could send DATA 0 followed by INVALID,
+> to avoid needing to add to the protocol. That should avoid
+> the spurious "verification of content failed".
+> 
+> But what causes the connection to get closed? It seems that
+> while the server sends VALID, the client never debugs that it received
+> it. Indeeed, the receiveMessage call that should receive it
+> fails because the handle is closed at that point. Seems that
+> this is caused by trying to receive 0 bytes as indicated by DATA
+> ending up closing the handle.