You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ga...@apache.org on 2022/05/26 05:40:09 UTC

[hadoop] branch trunk updated: HDFS-16561. Handle error returned by strtol

This is an automated email from the ASF dual-hosted git repository.

gaurava pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 0be1fde9626 HDFS-16561. Handle error returned by strtol
0be1fde9626 is described below

commit 0be1fde9626c58beffa02a036e00b784ca2b1b13
Author: Rishabh Sharma <35...@users.noreply.github.com>
AuthorDate: Thu May 26 11:09:49 2022 +0530

    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.
---
 .../main/native/libhdfspp/tests/tools/hdfs-chmod-mock.cc  |  9 +++++++++
 .../main/native/libhdfspp/tests/tools/hdfs-tool-tests.h   | 13 +++++++++++++
 .../main/native/libhdfspp/tools/hdfs-chmod/hdfs-chmod.cc  | 15 ++++++++++++++-
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-chmod-mock.cc b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-chmod-mock.cc
index e0df32cc2a1..3c4ea24e0db 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-chmod-mock.cc
+++ b/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];
diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-tests.h b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-tests.h
index 4fef261d0e7..8a1ae8cf0b7 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-tests.h
+++ b/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");
diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tools/hdfs-chmod/hdfs-chmod.cc b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tools/hdfs-chmod/hdfs-chmod.cc
index 4775860fb44..cd5aefabfcf 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tools/hdfs-chmod/hdfs-chmod.cc
+++ b/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 {


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org