This is an automated email from the ASF dual-hosted git repository. mboehm7 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/systemds.git
commit 43ed96efe29a6b3e46ad64179b1e4e07995cbcdf Author: sebwrede <[email protected]> AuthorDate: Thu Dec 31 20:08:01 2020 +0100 [SYSTEMDS-2774] Fix federated privacy exception handling Remove exception instances from FederatedResponse (do not leak information in stack traces) Add catch clauses for DMLPrivacyException and FederatedWorkerHandlerException Also remove unused constructors from DMLPrivacyException and add comments to FederatedWorkerHandlerException Closes #1054. --- .../federated/FederatedWorkerHandler.java | 55 +++++++++++++--------- .../federated/FederatedWorkerHandlerException.java | 10 +++- .../sysds/runtime/privacy/DMLPrivacyException.java | 9 ---- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandler.java b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandler.java index 7cbf421..e3ec403 100644 --- a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandler.java +++ b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandler.java @@ -20,7 +20,6 @@ package org.apache.sysds.runtime.controlprogram.federated; import java.io.BufferedReader; -import java.io.IOException; import java.io.InputStreamReader; import java.util.Arrays; @@ -157,9 +156,11 @@ public class FederatedWorkerHandler extends ChannelInboundHandlerAdapter { catch(DMLPrivacyException | FederatedWorkerHandlerException ex) { return new FederatedResponse(ResponseType.ERROR, ex); } - catch(Exception ex) { - return new FederatedResponse(ResponseType.ERROR, new FederatedWorkerHandlerException( - "Exception of type " + ex.getClass() + " thrown when processing request", ex)); + catch (Exception ex) { + String msg = "Exception of type " + ex.getClass() + " thrown when processing request"; + log.error(msg, ex); + return new FederatedResponse(ResponseType.ERROR, + new FederatedWorkerHandlerException(msg)); } } @@ -209,17 +210,16 @@ public class FederatedWorkerHandler extends ChannelInboundHandlerAdapter { fmt = FileFormat.safeValueOf(mtd.getString(DataExpression.FORMAT_TYPE)); } } - catch(Exception ex) { - throw new DMLRuntimeException(ex); + catch (DMLPrivacyException | FederatedWorkerHandlerException ex){ + throw ex; + } + catch (Exception ex) { + String msg = "Exception in reading metadata of: " + filename; + log.error(msg, ex); + throw new DMLRuntimeException(msg); } finally { - if(fs != null) - try { - fs.close(); - } - catch(IOException e) { - return new FederatedResponse(ResponseType.ERROR, id); - } + IOUtilFunctions.closeSilently(fs); } // put meta data object in symbol table, read on first operation @@ -302,9 +302,14 @@ public class FederatedWorkerHandler extends ChannelInboundHandlerAdapter { try { pb.execute(ec); // execute single instruction } + catch(DMLPrivacyException | FederatedWorkerHandlerException ex){ + throw ex; + } catch(Exception ex) { - return new FederatedResponse(ResponseType.ERROR, new FederatedWorkerHandlerException( - "Exception of type " + ex.getClass() + " thrown when processing EXEC_INST request", ex)); + String msg = "Exception of type " + ex.getClass() + " thrown when processing EXEC_INST request"; + log.error(msg, ex); + return new FederatedResponse(ResponseType.ERROR, + new FederatedWorkerHandlerException(msg)); } return new FederatedResponse(ResponseType.SUCCESS_EMPTY); } @@ -322,9 +327,13 @@ public class FederatedWorkerHandler extends ChannelInboundHandlerAdapter { try { return udf.execute(ec, inputs); } + catch(DMLPrivacyException | FederatedWorkerHandlerException ex){ + throw ex; + } catch(Exception ex) { - return new FederatedResponse(ResponseType.ERROR, new FederatedWorkerHandlerException( - "Exception of type " + ex.getClass() + " thrown when processing EXEC_UDF request", ex)); + String msg = "Exception of type " + ex.getClass() + " thrown when processing EXEC_UDF request"; + log.error(msg, ex); + return new FederatedResponse(ResponseType.ERROR, new FederatedWorkerHandlerException(msg)); } } @@ -332,9 +341,13 @@ public class FederatedWorkerHandler extends ChannelInboundHandlerAdapter { try { _ecm.clear(); } + catch(DMLPrivacyException | FederatedWorkerHandlerException ex){ + throw ex; + } catch(Exception ex) { - return new FederatedResponse(ResponseType.ERROR, new FederatedWorkerHandlerException( - "Exception of type " + ex.getClass() + " thrown when processing CLEAR request", ex)); + String msg = "Exception of type " + ex.getClass() + " thrown when processing CLEAR request"; + log.error(msg, ex); + return new FederatedResponse(ResponseType.ERROR, new FederatedWorkerHandlerException(msg)); } return new FederatedResponse(ResponseType.SUCCESS_EMPTY); } @@ -342,8 +355,8 @@ public class FederatedWorkerHandler extends ChannelInboundHandlerAdapter { private static void checkNumParams(int actual, int... expected) { if(Arrays.stream(expected).anyMatch(x -> x == actual)) return; - throw new DMLRuntimeException("FederatedWorkerHandler: Received wrong amount of params:" + " expected=" - + Arrays.toString(expected) + ", actual=" + actual); + throw new DMLRuntimeException("FederatedWorkerHandler: Received wrong amount of params:" + + " expected=" + Arrays.toString(expected) + ", actual=" + actual); } @Override diff --git a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandlerException.java b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandlerException.java index 77768b3..3d77d56 100644 --- a/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandlerException.java +++ b/src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandlerException.java @@ -37,7 +37,15 @@ public class FederatedWorkerHandlerException extends RuntimeException { public FederatedWorkerHandlerException(String msg) { super(msg); } - + + /** + * Create new instance of FederatedWorkerHandlerException with a message + * and a throwable representing the original cause of the exception. + * This constructor should not be used unless the throwable @param t + * does not expose any private data in any use case. + * @param msg message describing the exception + * @param t throwable representing the original cause of the exception + */ public FederatedWorkerHandlerException(String msg, Throwable t) { super(msg, t); } diff --git a/src/main/java/org/apache/sysds/runtime/privacy/DMLPrivacyException.java b/src/main/java/org/apache/sysds/runtime/privacy/DMLPrivacyException.java index 7e77b04..e60672a 100644 --- a/src/main/java/org/apache/sysds/runtime/privacy/DMLPrivacyException.java +++ b/src/main/java/org/apache/sysds/runtime/privacy/DMLPrivacyException.java @@ -27,15 +27,6 @@ import org.apache.sysds.runtime.DMLRuntimeException; public class DMLPrivacyException extends DMLRuntimeException { private static final long serialVersionUID = 1L; - - //prevent string concatenation of classname w/ stop message - private DMLPrivacyException(Exception e) { - super(e); - } - - private DMLPrivacyException(String string, Exception ex){ - super(string,ex); - } /** * This is the only valid constructor for DMLPrivacyException.
