Browse Source

HDFS-16561. Handle error returned by strtol

* strtol is used in hdfs-chmod.cc. The call
  to strtol could error out when an invalid
  input is provided.
* This PR handles the error given out by
   strtol.
Rishabh Sharma 3 years ago
parent
commit
0be1fde962

+ 9 - 0
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-chmod-mock.cc

@@ -55,6 +55,15 @@ void ChmodMock::SetExpectations(
         .WillOnce(testing::Return(true));
   }
 
+  if (*test_case_func == &PassInvalidPermissionsAndAPath<ChmodMock>) {
+    const auto arg1 = args[0];
+    const auto arg2 = args[1];
+
+    EXPECT_CALL(*this, HandlePath(arg1, false, arg2))
+        .Times(1)
+        .WillOnce(testing::Return(false));
+  }
+
   if (*test_case_func == &PassRecursivePermissionsAndAPath<ChmodMock>) {
     const auto arg1 = args[1];
     const auto arg2 = args[2];

+ 13 - 0
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-tests.h

@@ -175,6 +175,19 @@ template <class T> std::unique_ptr<T> PassPermissionsAndAPath() {
   return hdfs_tool;
 }
 
+template <class T> std::unique_ptr<T> PassInvalidPermissionsAndAPath() {
+  constexpr auto argc = 3;
+  static std::string exe("hdfs_tool_name");
+  static std::string arg1("123456789123456789123456789");
+  static std::string arg2("g/h/i");
+
+  static char *argv[] = {exe.data(), arg1.data(), arg2.data()};
+
+  auto hdfs_tool = std::make_unique<T>(argc, argv);
+  hdfs_tool->SetExpectations(PassInvalidPermissionsAndAPath<T>, {arg1, arg2});
+  return hdfs_tool;
+}
+
 template <class T> std::unique_ptr<T> PassRecursivePermissionsAndAPath() {
   constexpr auto argc = 4;
   static std::string exe("hdfs_tool_name");

+ 14 - 1
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tools/hdfs-chmod/hdfs-chmod.cc

@@ -140,8 +140,21 @@ bool Chmod::HandlePath(const std::string &permissions, const bool recursive,
   /*
    * strtol is reading the value with base 8, NULL because we are reading in
    * just one value.
+   *
+   * The strtol function may result in errors so check for that before
+   * typecasting.
    */
-  auto perm = static_cast<uint16_t>(strtol(permissions.c_str(), nullptr, 8));
+  errno = 0;
+  long result = strtol(permissions.c_str(), nullptr, 8);
+  bool all_0_in_permission = std::all_of(permissions.begin(), permissions.end(),
+                                         [](char c) { return c == '0'; });
+  /*
+   * The errno is set to ERANGE incase the string doesn't fit in long
+   * Also, the result is set to 0, in case conversion is not possible
+   */
+  if ((errno == ERANGE) || (!all_0_in_permission && result == 0))
+    return false;
+  auto perm = static_cast<uint16_t>(result);
   if (!recursive) {
     fs->SetPermission(uri.get_path(), perm, handler);
   } else {