ADC searches are not handled correctly

Bug #1010996 reported by maksis
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DC++
Critical
Unassigned
StrongDC++
Undecided
Unassigned

Bug Description

When responding to ADC searches, words found from upper level share directories are not removed from the search when searching from directories inside them (files in the same directory are handled correctly though). Due to this, the client doesn't send all search replies that it should.

The problem has been fixed in AirDC++ 2.30 by removing "StringSearch::List* cur" from void ShareManager::Directory::search(SearchResultList& aResults, AdcSearch& aStrings, StringList::size_type maxResults) and using aStrings.include instead.

maksis (maksis)
affects: dcplusplus → strongdc
maksis (maksis)
description: updated
Revision history for this message
eMTee (realprogger) wrote :

Here's a diff of the mentioned function to AirDC++ 2.30, this might make this report more understandable http://pastebin.com/Gc4Ef0tA

Revision history for this message
maksis (maksis) wrote :

no one is able to confirm this problem? it should be pretty straightforward to test

Revision history for this message
eMTee (realprogger) wrote :

That would require to understand the problem first, which I personally wasn't able to from your description. I tried to make a diff between AirDC++ and the head revison of DC++ so that might help others to figure out what is the actual problem.

Revision history for this message
maksis (maksis) wrote :

Okay, here's an example:

Another user is sharing a dir "/Video/CD1/" and you perform a search by using a search term "video cd1". You will receive results from that other user via NMDC hubs but not via ADC hubs. This is because in ADC hubs the client hits the word "video" from the parent directory name, but it's not removed from the search string list which is being used when searching from the subdirectories of the dir "Video". So basically it would still try to match both words "video" and "cd1" when matching the directory name "CD1" when it should only be matching "cd1".

If you do the changes from AirDC++, you should also receive the ADC results in the case above.

From a comment in ShareManager:

"Also, we want to avoid changing StringLists unless we absolutely have to --> this should only be done if a string has been matched in the directory name. This new stringlist should also be used in all descendants, but not the parents..."

Revision history for this message
Big Muscle (bigmuscle) wrote :

If it works correctly in NMDC, I did a quick code compare between ADC and NMDC method of ShareManager::Directory::search(...). The biggest difference I see is that nested directories are being compared against "cur" in NMDC but against "aStrings.include" in ADC. I think that "cur" can be modified by current directory but "aStrings" should stay unmodified through whole search process. And the nested directories could always get "cur" only.

Revision history for this message
maksis (maksis) wrote :

Here's a possible fix for this issue with minimal changes.

The patch will also fix a problem with excluded words, if there are no included words to match in the search (all of them were already matched on an upper level). Without the additional check, excludes wouldn't be checked at all for directories in those cases.

Revision history for this message
maksis (maksis) wrote :

Okay, I found another problem already.

From the protocol description: "Each filename (including the path to it) should be matched using case insensitive substring search as follows: match all AN, remove those that match any NO, and make sure the extension matches at least one of the EX (if it is present)."

I understand this in a way that also excluded words should be matched from the full path (which isn't the way how it currently works). Here's a new patch that won't continue to sub folders if the directory name contains excluded words.

Revision history for this message
poy (poy) wrote :

trying to make sense of the code, the following patch seems like it could fix the sub-dir search issues to me; can you check? basically change the list of strings to be passed to sub-dirs to "cur" (which is either the main one or a modified one), perform the search then set it back to "old" (the main term list).

=== modified file 'dcpp/ShareManager.cpp'
--- dcpp/ShareManager.cpp 2013-04-13 15:08:45 +0000
+++ dcpp/ShareManager.cpp 2013-07-30 18:25:42 +0000
@@ -1307,6 +1307,7 @@
   }
  }

+ aStrings.include = cur;
  for(auto l = directories.begin(); (l != directories.end()) && (aResults.size() < maxResults); ++l) {
   l->second->search(aResults, aStrings, maxResults);
  }

Changed in dcplusplus:
importance: Undecided → Critical
status: New → Confirmed
Revision history for this message
maksis (maksis) wrote :

Yeah, it does the same job as my first patch (I just got rid of the "cur" variable)

Revision history for this message
poy (poy) wrote :

in rev 3329.

in addition to your changes, i have merged the NMDC & ADC searching paths as they were so similar.

will need thorough testing.

Changed in dcplusplus:
status: Confirmed → Fix Committed
Revision history for this message
poy (poy) wrote :

Fixed in DC++ 0.830.

Changed in dcplusplus:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers