Setting a max candidate size limit (80 chars)

This prevents issues with auto-generated strings entering the database and screwing everything up. Fixes ycm-core/YouCompleteMe#520
ycm-core · Feb 16, 2016 · e6d0e49 · e6d0e49
1 parent 18693fb
commit e6d0e49
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 8 deletions.
diff --git a/cpp/ycm/CandidateRepository.cpp b/cpp/ycm/CandidateRepository.cpp
@@ -33,6 +33,15 @@ namespace YouCompleteMe {
 using boost::all;
 using boost::is_print;
 
+namespace {
+
+// We set a reasonable max limit to prevent issues with huge candidate strings
+// entering the database. Such large candidates are almost never desirable.
+const int MAX_CANDIDATE_SIZE = 80;
+
+}  // unnamed namespace
+
+
 boost::mutex CandidateRepository::singleton_mutex_;
 CandidateRepository *CandidateRepository::instance_ = NULL;
 
@@ -64,9 +73,7 @@ std::vector< const Candidate * > CandidateRepository::GetCandidatesForStrings(
 
     foreach ( const std::string & candidate_text, strings ) {
       const std::string &validated_candidate_text =
-        all( candidate_text, is_print( std::locale::classic() ) ) ?
-        candidate_text :
-        empty_;
+        ValidatedCandidateText( candidate_text );
 
       const Candidate *&candidate = GetValueElseInsert(
                                       candidate_holder_,
@@ -94,12 +101,16 @@ std::vector< const Candidate * > CandidateRepository::GetCandidatesForStrings(
     boost::lock_guard< boost::mutex > locker( holder_mutex_ );
 
     foreach ( const CompletionData & data, datas ) {
-      const Candidate *&candidate = GetValueElseInsert( candidate_holder_,
-                                                        data.original_string_,
-                                                        NULL );
+      const std::string &validated_candidate_text =
+        ValidatedCandidateText( data.original_string_ );
+
+      const Candidate *&candidate = GetValueElseInsert(
+                                      candidate_holder_,
+                                      validated_candidate_text,
+                                      NULL );
 
       if ( !candidate )
-        candidate = new Candidate( data.original_string_ );
+        candidate = new Candidate( validated_candidate_text );
 
       candidates.push_back( candidate );
     }
@@ -117,4 +128,15 @@ CandidateRepository::~CandidateRepository() {
   }
 }
 
+
+// Returns a ref to empty_ if candidate not valid.
+const std::string &CandidateRepository::ValidatedCandidateText(
+  const std::string &candidate_text ) {
+  if ( candidate_text.size() <= MAX_CANDIDATE_SIZE &&
+       all( candidate_text, is_print( std::locale::classic() ) ) )
+    return candidate_text;
+
+  return empty_;
+}
+
 } // namespace YouCompleteMe
diff --git a/cpp/ycm/CandidateRepository.h b/cpp/ycm/CandidateRepository.h
@@ -62,6 +62,8 @@ class CandidateRepository : boost::noncopyable {
   CandidateRepository() {};
   ~CandidateRepository();
 
+  const std::string &ValidatedCandidateText( const std::string &text );
+
   boost::mutex holder_mutex_;
 
   static boost::mutex singleton_mutex_;

diff --git a/cpp/ycm/IdentifierDatabase.cpp b/cpp/ycm/IdentifierDatabase.cpp
@@ -137,6 +137,7 @@ std::set< const Candidate * > &IdentifierDatabase::GetCandidateSet(
   return *candidates;
 }
 
+
 // WARNING: You need to hold the filetype_candidate_map_mutex_ before calling
 // this function and while using the returned set.
 void IdentifierDatabase::AddIdentifiersNoLock(
@@ -154,5 +155,4 @@ void IdentifierDatabase::AddIdentifiersNoLock(
 }
 
 
-
 } // namespace YouCompleteMe
diff --git a/cpp/ycm/tests/CandidateRepository_test.cpp b/cpp/ycm/tests/CandidateRepository_test.cpp
@@ -22,6 +22,32 @@
 
 namespace YouCompleteMe {
 
+TEST( CandidateRepositoryTest, Basic ) {
+  std::vector< std::string > inputs;
+  inputs.push_back( "foobar" );
+
+  CandidateRepository &repo = CandidateRepository::Instance();
+  std::vector< const Candidate * > candidates =
+    repo.GetCandidatesForStrings( inputs );
+
+  EXPECT_EQ( "foobar", candidates[ 0 ]->Text() );
+}
+
+
+TEST( CandidateRepositoryTest, TooLongCandidateSkipped ) {
+  std::vector< std::string > inputs;
+  inputs.push_back( std::string( 81, 'a' ) );  // this one is too long
+  inputs.push_back( std::string( 80, 'b' ) );  // this one is *just* right
+
+  CandidateRepository &repo = CandidateRepository::Instance();
+  std::vector< const Candidate * > candidates =
+    repo.GetCandidatesForStrings( inputs );
+
+  EXPECT_EQ( "", candidates[ 0 ]->Text() );
+  EXPECT_EQ( 'b', candidates[ 1 ]->Text()[ 0 ] );
+}
+
+
 TEST( CandidateRepositoryTest, EmptyCandidatesForUnicode ) {
   std::vector< std::string > inputs;
   inputs.push_back( "fooδιακριτικός" );