[
https://issues.apache.org/jira/browse/HDFS-16561?focusedWorklogId=770682&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-770682
]
ASF GitHub Bot logged work on HDFS-16561:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 16/May/22 07:14
Start Date: 16/May/22 07:14
Worklog Time Spent: 10m
Work Description: GauthamBanasandra commented on code in PR #4287:
URL: https://github.com/apache/hadoop/pull/4287#discussion_r873393779
##########
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tools/hdfs-chmod/hdfs-chmod.cc:
##########
@@ -140,8 +140,17 @@ 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);
+ /*
+ * 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) || result == 0) return false;
Review Comment:
One solution that I thought of was to use
[std::all_of](https://en.cppreference.com/w/cpp/algorithm/all_any_none_of) to
check if `permissions` contains only 0 and then bypass the `result == 0` if
that happens to be the case.
Issue Time Tracking
-------------------
Worklog Id: (was: 770682)
Time Spent: 2h (was: 1h 50m)
> Handle error returned by strtol
> -------------------------------
>
> Key: HDFS-16561
> URL: https://issues.apache.org/jira/browse/HDFS-16561
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: libhdfs++
> Affects Versions: 3.4.0
> Reporter: Gautham Banasandra
> Assignee: Gautham Banasandra
> Priority: Major
> Labels: pull-request-available
> Time Spent: 2h
> Remaining Estimate: 0h
>
> *strtol* is used in
> [hdfs-chmod.cc|https://github.com/apache/hadoop/blob/6dddbd42edd57cc26279c678756386a47c040af5/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tools/hdfs-chmod/hdfs-chmod.cc#L144].
> The call to strtol could error out when an invalid input is provided. Need
> to handle the error given out by strtol.
> Tasks to do -
> 1. Detect the error returned by strtol. The [strtol documentation
> |https://en.cppreference.com/w/cpp/string/byte/strtol]explains how to do so.
> 2. Return false to the caller if the error is detected.
> 3. Extend
> [this|https://github.com/apache/hadoop/blob/6dddbd42edd57cc26279c678756386a47c040af5/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-chmod-mock.cc]
> unit test and add a case which exercises this by passing an invalid input.
> Please refer to this PR to get more context on how this unit test is written
> - https://github.com/apache/hadoop/pull/3588.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
