Handle invalid input, add more tests for SourceHighlighter

In response to Finnur's @ 23624002

BUG=21734

Review URL: https://chromiumcodereview.appspot.com/23875013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222330 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 45 insertions, 5 deletions

diff --git a/chrome/browser/ui/webui/extensions/extension_error_handler.cc b/chrome/browser/ui/webui/extensions/extension_error_handler.cc
index 45bd71f..f89feb4 100644
--- a/chrome/browser/ui/webui/extensions/extension_error_handler.cc
+++ b/chrome/browser/ui/webui/extensions/extension_error_handler.cc
@@ -90,11 +90,11 @@ void ExtensionErrorHandler::HandleRequestFileSource(
 
   // Three required arguments: extension_id, path_suffix, and error_message.
   std::string extension_id;
-  base::FilePath::StringType path_suffix;
+  base::FilePath::StringType path_suffix_string;
   base::string16 error_message;
 
   if (!args->GetDictionary(0, &dict) ||
-      !dict->GetString(kPathSuffixKey, &path_suffix) ||
+      !dict->GetString(kPathSuffixKey, &path_suffix_string) ||
       !dict->GetString(ExtensionError::kExtensionIdKey, &extension_id) ||
       !dict->GetString(ExtensionError::kMessageKey, &error_message)) {
     NOTREACHED();
@@ -105,6 +105,13 @@ void ExtensionErrorHandler::HandleRequestFileSource(
       ExtensionSystem::Get(Profile::FromWebUI(web_ui()))->
           extension_service()->GetExtensionById(extension_id,
                                                 true /* include disabled */ );
+
+  // Under no circumstances should we ever need to reference a file outside of
+  // the extension's directory. If it tries to, abort.
+  base::FilePath path_suffix(path_suffix_string);
+  if (path_suffix.ReferencesParent())
+    return;
+
   base::FilePath path = extension->path().Append(path_suffix);
 
   // Setting the title and the error message is the same for all file types.
@@ -118,7 +125,7 @@ void ExtensionErrorHandler::HandleRequestFileSource(
   base::Closure closure;
   std::string* contents = NULL;
 
-  if (path_suffix == kManifestFilename) {
+  if (path_suffix_string == kManifestFilename) {
     std::string manifest_key;
     if (!dict->GetString(ManifestError::kManifestKeyKey, &manifest_key)) {
       NOTREACHED();
diff --git a/extensions/browser/file_highlighter.cc b/extensions/browser/file_highlighter.cc
index d3edeca..2cc669e 100644
--- a/extensions/browser/file_highlighter.cc
+++ b/extensions/browser/file_highlighter.cc
@@ -203,8 +203,19 @@ SourceHighlighter::~SourceHighlighter() {
 }
 
 void SourceHighlighter::Parse(size_t line_number) {
-  for (size_t i = 1; i < line_number; ++i)
-    start_ = contents_.find('\n', start_) + 1;
+  // If line 0 is requested, highlight nothing.
+  if (line_number == 0) {
+    start_ = contents_.size();
+    return;
+  }
+
+  for (size_t i = 1; i < line_number; ++i) {
+    start_ = contents_.find('\n', start_);
+    if (start_ == std::string::npos)
+      break;
+    start_ += 1;
+  }
+
   end_ = contents_.find('\n', start_);
 
   // If we went off the end of the string (i.e., the line number was invalid),
diff --git a/extensions/browser/file_highlighter_unittest.cc b/extensions/browser/file_highlighter_unittest.cc
index 6352be4..a449bf9 100644
--- a/extensions/browser/file_highlighter_unittest.cc
+++ b/extensions/browser/file_highlighter_unittest.cc
@@ -95,4 +95,26 @@ TEST(ManifestHighlighterUnitTest, ManifestHighlighterUnitTest) {
   EXPECT_EQ(kInternationalFeature, international_feature.GetFeature());
 }
 
+TEST(SouceHighlighterUnitTest, SourceHighlighterUnitTest) {
+  const char kBasicSourceFile[] = "line one\nline two\nline three";
+
+  SourceHighlighter basic1(kBasicSourceFile, 1u);
+  EXPECT_EQ("line one", basic1.GetFeature());
+  SourceHighlighter basic2(kBasicSourceFile, 2u);
+  EXPECT_EQ("line two", basic2.GetFeature());
+  SourceHighlighter basic3(kBasicSourceFile, 3u);
+  EXPECT_EQ("line three", basic3.GetFeature());
+
+  const char kNoNewlineSourceFile[] = "thisisonelonglinewithnobreaksinit";
+
+  SourceHighlighter full_line(kNoNewlineSourceFile, 1u);
+  EXPECT_EQ(kNoNewlineSourceFile, full_line.GetFeature());
+
+  SourceHighlighter line_zero(kNoNewlineSourceFile, 0u);
+  EXPECT_EQ(EmptyString(), line_zero.GetFeature());
+
+  SourceHighlighter out_of_bounds(kNoNewlineSourceFile, 2u);
+  EXPECT_EQ(EmptyString(), out_of_bounds.GetFeature());
+}
+
 }  // namespace extensions