Copilot commented on code in PR #9781:
URL: https://github.com/apache/ozone/pull/9781#discussion_r2819997353
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OMAdminProtocolServerSideImpl.java:
##########
@@ -141,10 +142,10 @@ public TriggerSnapshotDefragResponse
triggerSnapshotDefrag(
.setSuccess(true)
.setResult(result)
.build();
- } catch (Exception ex) {
+ } catch (IOException ex) {
return TriggerSnapshotDefragResponse.newBuilder()
.setSuccess(false)
- .setErrorMsg(ex.getMessage())
+ .setErrorMsg(ex.getMessage() == null ?
StringUtils.stringifyException(ex) : ex.getMessage())
.build();
Review Comment:
Using StringUtils.stringifyException(ex) as the fallback will embed the full
stack trace into the RPC/protobuf errorMsg. This can make responses very large
and can expose internal implementation details to clients. Consider falling
back to ex.toString() (or ex.getClass().getName()) and logging the exception
server-side instead, or truncate/sanitize the stack trace before returning it.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OMAdminProtocolServerSideImpl.java:
##########
@@ -104,7 +105,7 @@ public DecommissionOMResponse decommission(RpcController
controller,
} catch (IOException ex) {
return DecommissionOMResponse.newBuilder()
.setSuccess(false)
- .setErrorMsg(ex.getMessage())
+ .setErrorMsg(ex.getMessage() == null ?
StringUtils.stringifyException(ex) : ex.getMessage())
.build();
Review Comment:
Using StringUtils.stringifyException(ex) as the fallback will embed the full
stack trace into the RPC/protobuf errorMsg. This can make responses very large
and can expose internal implementation details to clients. Consider falling
back to ex.toString() (or ex.getClass().getName()) and logging the exception
server-side instead, or truncate/sanitize the stack trace before returning it.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OMAdminProtocolServerSideImpl.java:
##########
@@ -120,10 +121,10 @@ public CompactResponse compactDB(RpcController
controller, CompactRequest compac
// check if table exists. IOException is thrown if table is not found.
ozoneManager.getMetadataManager().getStore().getTable(compactRequest.getColumnFamily());
ozoneManager.compactOMDB(compactRequest.getColumnFamily());
- } catch (Exception ex) {
+ } catch (IOException ex) {
return CompactResponse.newBuilder()
.setSuccess(false)
- .setErrorMsg(ex.getMessage())
+ .setErrorMsg(ex.getMessage() == null ?
StringUtils.stringifyException(ex) : ex.getMessage())
.build();
Review Comment:
Using StringUtils.stringifyException(ex) as the fallback will embed the full
stack trace into the RPC/protobuf errorMsg. This can make responses very large
and can expose internal implementation details to clients. Consider falling
back to ex.toString() (or ex.getClass().getName()) and logging the exception
server-side instead, or truncate/sanitize the stack trace before returning it.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OMInterServiceProtocolServerSideImpl.java:
##########
@@ -68,7 +69,7 @@ public BootstrapOMResponse bootstrap(RpcController controller,
return BootstrapOMResponse.newBuilder()
.setSuccess(false)
.setErrorCode(ErrorCode.RATIS_BOOTSTRAP_ERROR)
- .setErrorMsg(ex.getMessage())
+ .setErrorMsg(ex.getMessage() == null ?
StringUtils.stringifyException(ex) : ex.getMessage())
Review Comment:
Using StringUtils.stringifyException(ex) as the fallback will embed the full
stack trace into the RPC/protobuf errorMsg. This can make responses very large
and can expose internal implementation details to clients. Consider falling
back to ex.toString() (or ex.getClass().getName()) and logging the exception
server-side instead, or truncate/sanitize the stack trace before returning it.
```suggestion
.setErrorMsg(ex.getMessage() == null ? ex.toString() :
ex.getMessage())
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java:
##########
@@ -1619,7 +1620,7 @@ public DecommissionScmResponseProto decommissionScm(
} catch (IOException ex) {
decommissionScmResponseBuilder
.setSuccess(false)
- .setErrorMsg(ex.getMessage());
+ .setErrorMsg(ex.getMessage() == null ?
StringUtils.stringifyException(ex) : ex.getMessage());
Review Comment:
Using StringUtils.stringifyException(ex) as the fallback will embed the full
stack trace into the decommission response errorMsg. This can make responses
very large and can expose internal implementation details to clients. Consider
falling back to ex.toString() (or ex.getClass().getName()) and logging the
exception server-side instead, or truncate/sanitize the stack trace before
returning it.
```suggestion
.setErrorMsg(ex.getMessage() == null ? ex.toString() :
ex.getMessage());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
