exceptionfactory commented on code in PR #41:
URL: https://github.com/apache/nifi-maven/pull/41#discussion_r1860922204
##########
src/main/java/org/apache/nifi/NarMojo.java:
##########
@@ -16,6 +16,7 @@
*/
package org.apache.nifi;
+import org.apache.commons.codec.digest.DigestUtils;
Review Comment:
This appears to be coming through a transitive dependency, and as such, it
should be declared explicitly. On the other hand, it may be cleaner to simply
use the `MessageDigest` component directly without this utility class.
##########
src/main/java/org/apache/nifi/NarMojo.java:
##########
@@ -1053,6 +1065,29 @@ private void makeNar() throws MojoExecutionException {
final File extensionDocsFile = narResult.getExtensionDocsFile();
if (extensionDocsFile != null && !skipDocGeneration) {
projectHelper.attachArtifact(project, "xml",
"nar-extension-manifest", extensionDocsFile);
+ addNarSha512(narFile, extensionDocsFile);
+ }
+ }
+
+ private void addNarSha512(final File narFile, final File
extensionDocsFile) {
+ try {
+ final String narSha512 = DigestUtils.sha512Hex(new
FileInputStream(narFile));
+
+ DocumentBuilderFactory documentBuilderFactory =
DocumentBuilderFactory.newInstance();
+ DocumentBuilder documentBuilder =
documentBuilderFactory.newDocumentBuilder();
Review Comment:
The default settings for DocumentBuilder are not secure and leave it open to
external entity vulnerabilities.
##########
src/main/java/org/apache/nifi/NarMojo.java:
##########
@@ -1053,6 +1065,29 @@ private void makeNar() throws MojoExecutionException {
final File extensionDocsFile = narResult.getExtensionDocsFile();
if (extensionDocsFile != null && !skipDocGeneration) {
projectHelper.attachArtifact(project, "xml",
"nar-extension-manifest", extensionDocsFile);
+ addNarSha512(narFile, extensionDocsFile);
Review Comment:
This approach of reading the entire manifest and appending the SHA-512 hash
seems inefficient. Understanding that introducing the hashing earlier in the
process would require some refactoring, did you consider alternative approaches?
##########
src/main/java/org/apache/nifi/NarMojo.java:
##########
@@ -1053,6 +1065,29 @@ private void makeNar() throws MojoExecutionException {
final File extensionDocsFile = narResult.getExtensionDocsFile();
if (extensionDocsFile != null && !skipDocGeneration) {
projectHelper.attachArtifact(project, "xml",
"nar-extension-manifest", extensionDocsFile);
+ addNarSha512(narFile, extensionDocsFile);
+ }
+ }
+
+ private void addNarSha512(final File narFile, final File
extensionDocsFile) {
+ try {
+ final String narSha512 = DigestUtils.sha512Hex(new
FileInputStream(narFile));
Review Comment:
The wrapping `FileInputStream` does not appear necessary, and the method
does not close the InputStream, so this approach should be changed.
--
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]
