[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1583: MINIFICPP-1719 Replace LibreSSL with OpenSSL 3.1
szaszm commented on code in PR #1583:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1583#discussion_r1246256189
##
extensions/standard-processors/processors/HashContent.h:
##
@@ -49,21 +48,25 @@ namespace { // NOLINT
HashReturnType ret_val;
ret_val.second = 0;
std::array buffer{};
-MD5_CTX context;
-MD5_Init(&context);
+EVP_MD_CTX *context = EVP_MD_CTX_new();
+const auto guard = gsl::finally([&context]() {
+ EVP_MD_CTX_free(context);
+});
+const EVP_MD *md = EVP_md5();
+EVP_DigestInit_ex(context, md, nullptr);
Review Comment:
Is there any reason not to inline the digest type call?
```suggestion
EVP_DigestInit_ex(context, EVP_md5(), nullptr);
```
##
libminifi/src/core/state/Value.cpp:
##
@@ -34,25 +34,29 @@ const std::type_index Value::BOOL_TYPE =
std::type_index(typeid(bool));
const std::type_index Value::DOUBLE_TYPE = std::type_index(typeid(double));
const std::type_index Value::STRING_TYPE =
std::type_index(typeid(std::string));
-void hashNode(const SerializedResponseNode& node, SHA512_CTX& ctx) {
- SHA512_Update(&ctx, node.name.c_str(), node.name.length());
+void hashNode(const SerializedResponseNode& node, EVP_MD_CTX* ctx) {
Review Comment:
If ctx is expected to be valid/not-null, then keeping it as a reference
would document this on the interface.
##
extensions/standard-processors/processors/HashContent.h:
##
@@ -49,21 +48,25 @@ namespace { // NOLINT
HashReturnType ret_val;
ret_val.second = 0;
std::array buffer{};
-MD5_CTX context;
-MD5_Init(&context);
+EVP_MD_CTX *context = EVP_MD_CTX_new();
+const auto guard = gsl::finally([&context]() {
+ EVP_MD_CTX_free(context);
+});
+const EVP_MD *md = EVP_md5();
+EVP_DigestInit_ex(context, md, nullptr);
size_t ret = 0;
do {
ret = stream->read(buffer);
if (ret > 0) {
-MD5_Update(&context, buffer.data(), ret);
+EVP_DigestUpdate(context, buffer.data(), ret);
ret_val.second += gsl::narrow(ret);
}
} while (ret > 0);
if (ret_val.second > 0) {
- std::array digest{};
- MD5_Final(reinterpret_cast(digest.data()), &context);
+ std::array digest{};
+ EVP_DigestFinal_ex(context, reinterpret_cast(digest.data()), nullptr);
Review Comment:
The array is sized for SHA512 now. Did you verify that the resulting hash is
trimmed to MD5 size?
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1583: MINIFICPP-1719 Replace LibreSSL with OpenSSL 3.1
szaszm commented on code in PR #1583:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1583#discussion_r1229788131
##
README.md:
##
@@ -151,6 +151,10 @@ and rebuild.
* Lua and development headers -- Required if Lua support is enabled
* libgps-dev -- Required if building libGPS support
* Zlib headers
+* perl -- Required for OpenSSL configuration
+* NASM -- Required for OpenSSL only on Windows
+
+**NOTE** On Windows if Strawberry Perl is used the
`${StrawberryPerlRoot}\c\bin` directory should not be part of the %PATH%
variable as Strawberry Perl's patch.exe will be found as the patch executable
in the configure phase instead if the git patch executable.
Review Comment:
Cpack is used for MSI generation. It builds WIX files and calls WIX toolset
tools to create an MSI.
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1583: MINIFICPP-1719 Replace LibreSSL with OpenSSL 3.1
szaszm commented on code in PR #1583:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1583#discussion_r1222987263
##
NOTICE:
##
Review Comment:
Could you update the copyright year near the top?
##
bootstrap.sh:
##
@@ -338,6 +338,9 @@ add_option PROCFS_ENABLED ${TRUE} "ENABLE_PROCFS"
add_option PROMETHEUS_ENABLED ${FALSE} "ENABLE_PROMETHEUS"
+add_option OPENSSL_ENABLED ${TRUE} "OPENSSL_OFF"
+add_dependency OPENSSL_ENABLED "opensslbuild"
Review Comment:
why call it "opensslbuild" instead of just "openssl", like the other
dependencies?
##
cmake/GoogleCloudCpp.cmake:
##
@@ -40,3 +40,9 @@ FetchContent_MakeAvailable(google-cloud-cpp)
if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION
VERSION_GREATER_EQUAL "14.0.0" )
target_compile_options(google_cloud_cpp_common PUBLIC
-Wno-error=deprecated-pragma)
endif()
+
+if (WIN32)
+target_compile_options(google_cloud_cpp_storage PUBLIC /wd4996)
+else()
+target_compile_options(google_cloud_cpp_storage PUBLIC
-Wno-error=deprecated-declarations)
Review Comment:
Would you mind adding a note with an explanation or related error messages?
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1583: MINIFICPP-1719 Replace LibreSSL with OpenSSL 3.1
szaszm commented on code in PR #1583:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1583#discussion_r1222986101
##
README.md:
##
@@ -151,6 +151,10 @@ and rebuild.
* Lua and development headers -- Required if Lua support is enabled
* libgps-dev -- Required if building libGPS support
* Zlib headers
+* perl -- Required for OpenSSL configuration
+* NASM -- Required for OpenSSL only on Windows
+
+**NOTE** On Windows if Strawberry Perl is used the
`${StrawberryPerlRoot}\c\bin` directory should not be part of the %PATH%
variable as Strawberry Perl's patch.exe will be found as the patch executable
in the configure phase instead if the git patch executable.
Review Comment:
If we're adding warnings like this, we should add another one about
Chocolatey's cpack conflicting with CMake cpack. One could also solve the
problem by moving the build utils' PATH entries before the conflicting ones, at
least in the terminals used to build.
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
