XMLTransformer: Fix import of unwanted identities

Notice that this was not a severe security problem: As part of its design goals, the Score computation algorithm will ignore Trusts of Identitys for which shouldFetchIdentity() is false. So the unwanted Identitys did only bloat the database a bit, their Trusts were not able to affect the Score view of any OwnIdentity.
hyphanet · Mar 10, 2015 · ec38ccb · ec38ccb
1 parent 6b8c945
commit ec38ccb
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 9 deletions.
diff --git a/src/plugins/WebOfTrust/IdentityFetcher.java b/src/plugins/WebOfTrust/IdentityFetcher.java
@@ -795,8 +795,11 @@ public void onFound(USK origUSK, long edition, FetchResult result) {
                     // If mRequests doesn't contain the request thats not necessarily an error:
                     // This thread might not have gotten the locks before the thread which
                     // terminated the request.
-                    // But we MUST return here for sure because importIdentity() will NOT check
-                    // whether the identity is wanted.
+                    // Notice: This check can have false negatives: The identity might have a
+				    // pending AbortFetchCommand which was not processed yet. So mRequests can
+				    // still contain a request for the identity even though we should not fetch it.
+				    // Thus, the XMLTransformer will have to also check for whether the identity is
+				    // actually wanted.
 					return;
 				}
 

diff --git a/src/plugins/WebOfTrust/XMLTransformer.java b/src/plugins/WebOfTrust/XMLTransformer.java
@@ -395,13 +395,19 @@ public void importIdentity(FreenetURI identityURI, InputStream xmlInputStream) {
 
 			Logger.normal(this, "Importing parsed XML for " + identity);
 
-            // This is only an assert() and not an if() because it's an expensive database query and
-            // because it won't cause much damage if we import an unwanted identity: As part of its
-            // design goals, the Score computation algorithm will ignore Trusts of Identitys for
-            // which shouldFetchIdentity() is false. So the unwanted Identity will only bloat the
-            // database a bit, its Trusts won't be able to affect the Score view of any OwnIdentity.
-            assert mWoT.shouldFetchIdentity(identity)
-                 : "importIdentity() called for unwanted identity: " + identity;
+			// When shouldFetchIdentity() changes from true to false due to an identity becoming
+			// distrusted, this change will not cause the IdentityFetcher to abort the fetch
+			// immediately: It queues the command to abort the fetch, and processes commands after
+			// some seconds. Thus, it is possible that the IdentityFetcher calls this function to
+			// import an identity which is not actually wanted anymore. So we must check whether the
+			// identity is really still wanted.
+            if(!mWoT.shouldFetchIdentity(identity)) {
+                Logger.normal(this,
+                    "importIdentity() called for unwanted identity, probably because the "
+                  + "IdentityFetcher has not processed the AbortFetchCommand yet, not importing: "
+                  + identity);
+                return;
+            }
 
 			long newEdition = identityURI.getEdition();
 			if(identity.getEdition() > newEdition) {