seanjmullan commented on code in PR #271: URL: https://github.com/apache/santuario-xml-security-java/pull/271#discussion_r1576208433
########## src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java: ########## @@ -140,7 +140,7 @@ public static OAEPParameterSpec constructOAEPParameters( public static MGF1ParameterSpec constructMGF1Parameter(String mgh1AlgorithmURI) { LOG.log(Level.DEBUG, "Creating MGF1ParameterSpec for [{0}]", mgh1AlgorithmURI); if (mgh1AlgorithmURI == null || mgh1AlgorithmURI.isEmpty()) { - LOG.log(Level.WARNING,"MGF1 algorithm URI is null or empty. Using SHA-1 as default."); + LOG.log(Level.WARNING, "MGF1 algorithm URI is null or empty. Using SHA-1 as default."); Review Comment: I think the XML Encryption recommendation made a mistake to allow defaults for algorithms, but I don't think we need to necessarily adhere to that. That said, this could introduce interop issues if we treat a missing DigestMethod as SHA-256 instead of SHA-1. Instead, we could require the application to always specify the MGF digest method and no longer allow null to be passed in as the parameter. This may be best handled as a separate issue so that all APIs which have defaults are checked - for example, RSA-OAEP also defaults to SHA-1 for the digest algorithm. -- 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: dev-unsubscr...@santuario.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org