From 0540e987b36af281406136ceb89a60434a5374eb Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 1 Dec 2020 16:05:55 -0400 Subject: [PATCH] improve p2p protocol handling of requested object not available Avoid spurious "verification of content failed" message when downloading content from a ssh or tor remote fails due to the remote no longer having a copy of the content. The P2P protocol already handled this case by sending DATA 0, followed by VALID. But VALID was not really right, because the data is not the requested data. So, send DATA 0, followed by INVALID. Old versions of git-annex handle INVALID the same as VALID in this case. Now new versions avoid displaying an incorrect message. It would be better for the P2P protocol to have a different way to indicate this, like perhaps sending INVALID without DATA. But that would be a breaking change and need a new protocol verison. Since INVALID already is part of the protocol and already needs to be handled, using it for this special case too seems ok, and avoids the complication of another protocol version. This commit was sponsored by Jochen Bartl on Patreon. --- CHANGELOG | 3 +++ P2P/Annex.hs | 8 ++++++++ P2P/Protocol.hs | 4 +++- ...otocol_misbehavior_when_location_log_out_of_date.mdwn | 9 ++++++++- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 18890f78f0..85a60e66d3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,9 @@ git-annex (8.20201128) UNRELEASED; urgency=medium 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. + * Avoid spurious "verification of content failed" message when downloading + content from a ssh or tor remote fails due to the remote no longer + having a copy of the content. -- Joey Hess Mon, 30 Nov 2020 12:55:49 -0400 diff --git a/P2P/Annex.hs b/P2P/Annex.hs index 41f5f3b5dd..d107f6ef3b 100644 --- a/P2P/Annex.hs +++ b/P2P/Annex.hs @@ -172,6 +172,14 @@ runLocal runst runner a = case a of runner validitycheck >>= \case Right (Just Valid) -> return (rightsize, UnVerified) + Right (Just Invalid) | l == 0 -> + -- Special case, for when + -- content was not + -- available to send, + -- which is indicated by + -- sending 0 bytes and + -- Invalid. + return (False, UnVerified) _ -> do -- Invalid, or old protocol -- version. Validity is not diff --git a/P2P/Protocol.hs b/P2P/Protocol.hs index e9895d3de4..bc340e1c76 100644 --- a/P2P/Protocol.hs +++ b/P2P/Protocol.hs @@ -508,13 +508,15 @@ serveAuthed servermode myuuid = void $ serverLoop handler sendContent :: Key -> AssociatedFile -> Offset -> MeterUpdate -> Proto Bool sendContent key af offset@(Offset n) p = go =<< local (contentSize key) where - go Nothing = sender (Len 0) L.empty (return Valid) go (Just (Len totallen)) = do let len = totallen - n if len <= 0 then sender (Len 0) L.empty (return Valid) else local $ readContent key af offset $ sender (Len len) + -- Content not available to send. Indicate this by sending + -- empty data and indlicate it's invalid. + go Nothing = sender (Len 0) L.empty (return Invalid) sender len content validitycheck = do let p' = offsetMeterUpdate p (toBytesProcessed n) net $ sendMessage (DATA len) 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 9e967a2000..5e5aa74721 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 @@ -35,9 +35,16 @@ closes the connection, the next move fails when it should not need to. > to avoid needing to add to the protocol. That should avoid > the spurious "verification of content failed". > +> > Done and it did. +> > 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. +> ending up closing the handle. Another case of it involved getting +> an empty file followed by a second file. +> +> > This bug is fixed. + +[[done]] --[[Joey]] -- 2.30.2