Improve error handling for ax_dump_tree tool

ax_dump_tree tool sets siltently include-all filter if the filter file
path is relative or mistyped, and ignores silently unrecognized filters,
which makes harder to figure out why your filters don't work

Bug: None
Change-Id: I871350d67c9dc945ef4543eb11bb51c680aa025f
AX-Relnotes: n/a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2399123
Commit-Queue: Alexander Surkov <asurkov@igalia.com>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805419}

tools/accessibility/inspect/ax_tree_server.cc

@@ -59,6 +59,10 @@ void AXTreeServer::Run(BuildTree build_tree,
   // Set filters.
   std::vector<AccessibilityTreeFormatter::PropertyFilter> filters =
       GetPropertyFilters(filters_path);
+  if (filters.empty()) {
+    LOG(ERROR) << "Failed to parse filters";
+    return;
+  }
   formatter->SetPropertyFilters(filters);
 
   // Get accessibility tree as a nested dictionary.
@@ -75,38 +79,42 @@ void AXTreeServer::Run(BuildTree build_tree,
 
 std::vector<AccessibilityTreeFormatter::PropertyFilter>
 AXTreeServer::GetPropertyFilters(const base::FilePath& filters_path) {
-  std::vector<AccessibilityTreeFormatter::PropertyFilter> filters;
-  if (!filters_path.empty()) {
-    std::string raw_filters_text;
-    base::ScopedAllowBlockingForTesting allow_io_for_test_setup;
-    if (base::ReadFileToString(filters_path, &raw_filters_text)) {
-      for (const std::string& line :
-           base::SplitString(raw_filters_text, "\n", base::TRIM_WHITESPACE,
-                             base::SPLIT_WANT_ALL)) {
-        if (base::StartsWith(line, kAllowOptEmptyStr,
-                             base::CompareCase::SENSITIVE)) {
-          filters.emplace_back(
-              line.substr(strlen(kAllowOptEmptyStr)),
-              AccessibilityTreeFormatter::PropertyFilter::ALLOW_EMPTY);
-        } else if (base::StartsWith(line, kAllowOptStr,
-                                    base::CompareCase::SENSITIVE)) {
-          filters.emplace_back(
-              line.substr(strlen(kAllowOptStr)),
-              AccessibilityTreeFormatter::PropertyFilter::ALLOW);
-        } else if (base::StartsWith(line, kDenyOptStr,
-                                    base::CompareCase::SENSITIVE)) {
-          filters.emplace_back(
-              line.substr(strlen(kDenyOptStr)),
-              AccessibilityTreeFormatter::PropertyFilter::DENY);
-        }
-      }
-    }
-  }
-  if (filters.empty()) {
-    filters = {AccessibilityTreeFormatter::PropertyFilter(
+  if (filters_path.empty()) {
+    return {AccessibilityTreeFormatter::PropertyFilter(
         "*", AccessibilityTreeFormatter::PropertyFilter::ALLOW)};
   }
 
+  std::string raw_filters_text;
+  base::ScopedAllowBlockingForTesting allow_io_for_test_setup;
+  if (!base::ReadFileToString(filters_path, &raw_filters_text)) {
+    LOG(ERROR) << "Failed to open filters file " << filters_path
+               << ". Note: path traversal components ('..') are not allowed "
+                  "for security reasons";
+    return {};
+  }
+
+  std::vector<AccessibilityTreeFormatter::PropertyFilter> filters;
+  for (const std::string& line :
+       base::SplitString(raw_filters_text, "\n", base::TRIM_WHITESPACE,
+                         base::SPLIT_WANT_ALL)) {
+    if (base::StartsWith(line, kAllowOptEmptyStr,
+                         base::CompareCase::SENSITIVE)) {
+      filters.emplace_back(
+          line.substr(strlen(kAllowOptEmptyStr)),
+          AccessibilityTreeFormatter::PropertyFilter::ALLOW_EMPTY);
+    } else if (base::StartsWith(line, kAllowOptStr,
+                                base::CompareCase::SENSITIVE)) {
+      filters.emplace_back(line.substr(strlen(kAllowOptStr)),
+                           AccessibilityTreeFormatter::PropertyFilter::ALLOW);
+    } else if (base::StartsWith(line, kDenyOptStr,
+                                base::CompareCase::SENSITIVE)) {
+      filters.emplace_back(line.substr(strlen(kDenyOptStr)),
+                           AccessibilityTreeFormatter::PropertyFilter::DENY);
+    } else if (!line.empty()) {
+      LOG(ERROR) << "Unrecognized filter instruction at line: " << line;
+      return {};
+    }
+  }
   return filters;
 }