You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/09/18 17:07:13 UTC
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #886: MINIFICPP-1323 Encrypt sensitive properties
szaszm commented on a change in pull request #886:
URL: https://github.com/apache/nifi-minifi-cpp/pull/886#discussion_r490904072
##########
File path: encrypt-config/CMakeLists.txt
##########
@@ -0,0 +1,30 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+cmake_minimum_required(VERSION 3.7)
+project(encrypt-config)
Review comment:
I think this is part if the minifi project, not a separate one. Creating a new executable target should be enough.
##########
File path: encrypt-config/tests/CMakeLists.txt
##########
@@ -0,0 +1,44 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+
+file(GLOB ENCRYPT_CONFIG_TESTS "*.cpp")
+
+file(GLOB ENCRYPT_CONFIG_SOURCES "${CMAKE_SOURCE_DIR}/encrypt-config/*.cpp")
Review comment:
See the note at https://cmake.org/cmake/help/latest/command/file.html#glob
##########
File path: encrypt-config/CMakeLists.txt
##########
@@ -0,0 +1,30 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+cmake_minimum_required(VERSION 3.7)
+project(encrypt-config)
+set(VERSION, "0.1")
+
+file(GLOB ENCRYPT_CONFIG_FILES "*.cpp")
+add_executable(encrypt-config "${ENCRYPT_CONFIG_FILES}")
+include_directories(../libminifi/include ../thirdparty/cxxopts/include)
Review comment:
Normally, we should use `target_include_directories` to avoid cluttering the global include directory list. See the note at the bottom of the [include_directories documentation](https://cmake.org/cmake/help/latest/command/include_directories.html).
In this case, the linked targets should automatically propagate their required include directories, so it should work automatically. If it doesn't, then we need to fix those targets.
```suggestion
```
##########
File path: encrypt-config/EncryptConfig.cpp
##########
@@ -0,0 +1,146 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "EncryptConfig.h"
+
+#include "cxxopts.hpp"
+
+#include <stdexcept>
+
+#include "ConfigFile.h"
+#include "utils/file/FileUtils.h"
+#include "utils/OptionalUtils.h"
+
+namespace {
+const char* CONF_DIRECTORY_NAME = "conf";
+const char* BOOTSTRAP_FILE_NAME = "bootstrap.conf";
+const char* MINIFI_PROPERTIES_FILE_NAME = "minifi.properties";
+const char* ENCRYPTION_KEY_PROPERTY_NAME = "nifi.bootstrap.sensitive.key";
+const size_t ENCRYPTION_KEY_SIZE = 32;
Review comment:
These all could be `constexpr`. Currently the C strings are not top-level `const`.
##########
File path: encrypt-config/ConfigFile.h
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "utils/EncryptionUtils.h"
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+class ConfigLine {
+public:
+ ConfigLine(std::string line);
+ ConfigLine(const std::string& key, const std::string& value);
+
+ void updateValue(const std::string& value);
+
+ std::string line_;
+ std::string key_;
+ std::string value_;
+};
+
+inline bool operator==(const ConfigLine& left, const ConfigLine& right) {
+ return left.line_ == right.line_;
+}
+
+class ConfigFile {
+public:
+ ConfigFile(const std::string& file_path);
+
+ utils::optional<std::string> getValue(const std::string& key) const;
+ void update(const std::string& key, const std::string& value);
+ void insertAfter(const std::string& after_key, const std::string& key, const std::string& value);
+ void append(const std::string& key, const std::string& value);
+ int erase(const std::string& key);
+
+ void writeTo(const std::string& file_path) const;
+
+ size_t size() const { return config_lines_.size(); }
+
+ int encryptSensitiveProperties(const utils::crypto::Bytes& encryption_key);
Review comment:
This and `getSensitiveProperties` should be independent from `ConfigFile`, because encryption is a new, unrelated responsibility to config file processing, i.e. clear violation of SRP.
##########
File path: encrypt-config/ConfigFile.cpp
##########
@@ -0,0 +1,195 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConfigFile.h"
+
+#include <fstream>
+
+#include "utils/StringUtils.h"
+
+namespace {
+const std::array<const char*, 2> DEFAULT_SENSITIVE_PROPERTIES{"nifi.security.client.pass.phrase",
+ "nifi.rest.api.password"};
+const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME = "nifi.sensitive.props.additional.keys";
Review comment:
Add `constexpr` or at least top-level `const` to the C string.
##########
File path: encrypt-config/ConfigFile.cpp
##########
@@ -0,0 +1,195 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConfigFile.h"
+
+#include <fstream>
+
+#include "utils/StringUtils.h"
+
+namespace {
+const std::array<const char*, 2> DEFAULT_SENSITIVE_PROPERTIES{"nifi.security.client.pass.phrase",
+ "nifi.rest.api.password"};
+const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME = "nifi.sensitive.props.additional.keys";
+} // namespace
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+ConfigLine::ConfigLine(std::string line) : line_(line) {
+ line = utils::StringUtils::trim(line);
+ if (line.empty() || line[0] == '#') { return; }
+
+ size_t index_of_first_equals_sign = line.find('=');
+ if (index_of_first_equals_sign == std::string::npos) { return; }
+
+ std::string key = utils::StringUtils::trim(line.substr(0, index_of_first_equals_sign));
+ if (key.empty()) { return; }
+
+ key_ = key;
+ value_ = utils::StringUtils::trim(line.substr(index_of_first_equals_sign + 1));
+}
+
+ConfigLine::ConfigLine(const std::string& key, const std::string& value)
+ : line_{key + "=" + value}, key_{key}, value_{value} {
+}
+
+void ConfigLine::updateValue(const std::string& value) {
+ auto pos = line_.find('=');
+ if (pos != std::string::npos) {
+ line_.replace(pos + 1, std::string::npos, value);
+ value_ = value;
+ } else {
+ throw std::invalid_argument{"Cannot update value in config line: it does not contain an = sign!"};
+ }
+}
+
+ConfigFile::ConfigFile(const std::string& file_path) {
+ std::ifstream file{file_path};
Review comment:
Since this is just reading from a file, making it take an `std::istream&` (+ a delegating ctor for rvalues) would make it more generic and more strongly typed at the same time, with little effort.
##########
File path: encrypt-config/ConfigFile.h
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "utils/EncryptionUtils.h"
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+class ConfigLine {
+public:
+ ConfigLine(std::string line);
+ ConfigLine(const std::string& key, const std::string& value);
+
+ void updateValue(const std::string& value);
+
+ std::string line_;
+ std::string key_;
+ std::string value_;
+};
+
+inline bool operator==(const ConfigLine& left, const ConfigLine& right) {
+ return left.line_ == right.line_;
Review comment:
I would define this equivalence in terms of key and value, not the whole line, so that this
```
random.property = 42
```
and this
```
random.property=42
```
are equivalent.
##########
File path: encrypt-config/tests/CMakeLists.txt
##########
@@ -0,0 +1,44 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+
+file(GLOB ENCRYPT_CONFIG_TESTS "*.cpp")
+
+file(GLOB ENCRYPT_CONFIG_SOURCES "${CMAKE_SOURCE_DIR}/encrypt-config/*.cpp")
+list(REMOVE_ITEM ENCRYPT_CONFIG_SOURCES "${CMAKE_SOURCE_DIR}/encrypt-config/EncryptConfigMain.cpp")
+
+set(ENCRYPT_CONFIG_TEST_COUNT 0)
+foreach(testfile ${ENCRYPT_CONFIG_TESTS})
+ get_filename_component(testfilename "${testfile}" NAME_WE)
+ add_executable(${testfilename} "${testfile}" ${ENCRYPT_CONFIG_SOURCES})
+
+ target_include_directories(${testfilename} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/thirdparty/catch")
+ target_include_directories(${testfilename} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/encrypt-config")
+ target_include_directories(${testfilename} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include")
+ target_include_directories(${testfilename} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/test")
+ target_include_directories(${testfilename} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/thirdparty/cxxopts/include")
Review comment:
[`target_link_libraries`](https://cmake.org/cmake/help/v3.7/command/target_link_libraries.html) should propagate these automatically as "usage requirements".
```suggestion
```
##########
File path: encrypt-config/ConfigFile.h
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "utils/EncryptionUtils.h"
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+class ConfigLine {
+public:
+ ConfigLine(std::string line);
+ ConfigLine(const std::string& key, const std::string& value);
+
+ void updateValue(const std::string& value);
+
+ std::string line_;
+ std::string key_;
+ std::string value_;
+};
+
+inline bool operator==(const ConfigLine& left, const ConfigLine& right) {
+ return left.line_ == right.line_;
+}
+
+class ConfigFile {
+public:
+ ConfigFile(const std::string& file_path);
+
+ utils::optional<std::string> getValue(const std::string& key) const;
+ void update(const std::string& key, const std::string& value);
+ void insertAfter(const std::string& after_key, const std::string& key, const std::string& value);
+ void append(const std::string& key, const std::string& value);
Review comment:
`set` is a common mutation, we should have one IMO. It should `update` if the key is present or `append` if it's not.
##########
File path: encrypt-config/ConfigFile.cpp
##########
@@ -0,0 +1,195 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConfigFile.h"
+
+#include <fstream>
+
+#include "utils/StringUtils.h"
+
+namespace {
+const std::array<const char*, 2> DEFAULT_SENSITIVE_PROPERTIES{"nifi.security.client.pass.phrase",
+ "nifi.rest.api.password"};
+const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME = "nifi.sensitive.props.additional.keys";
+} // namespace
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+ConfigLine::ConfigLine(std::string line) : line_(line) {
+ line = utils::StringUtils::trim(line);
+ if (line.empty() || line[0] == '#') { return; }
+
+ size_t index_of_first_equals_sign = line.find('=');
+ if (index_of_first_equals_sign == std::string::npos) { return; }
+
+ std::string key = utils::StringUtils::trim(line.substr(0, index_of_first_equals_sign));
+ if (key.empty()) { return; }
+
+ key_ = key;
+ value_ = utils::StringUtils::trim(line.substr(index_of_first_equals_sign + 1));
+}
+
+ConfigLine::ConfigLine(const std::string& key, const std::string& value)
+ : line_{key + "=" + value}, key_{key}, value_{value} {
Review comment:
This could use pass-by-value and move to prevent a few unnecessary allocations and use `StringUtils::join_pack` to avoid one more.
```suggestion
ConfigLine::ConfigLine(std::string key, std::string value)
: line_{utils::StringUtils::join_pack(key, "=", value)}, key_{std::move(key)}, value_{std::move(value)} {
```
##########
File path: encrypt-config/ConfigFile.h
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "utils/EncryptionUtils.h"
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+class ConfigLine {
+public:
+ ConfigLine(std::string line);
+ ConfigLine(const std::string& key, const std::string& value);
+
+ void updateValue(const std::string& value);
+
+ std::string line_;
+ std::string key_;
+ std::string value_;
Review comment:
These should be private because there is an invariant to be enforced. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-struct
Alternatively, we should not keep the original line, only the key and value. In this case, the class essentially becomes `std::pair<std::string, std::string>`.
```suggestion
std::string key() const { return key_; }
std::string value() const { return value_; }
private:
std::string line_;
std::string key_;
std::string value_;
```
##########
File path: encrypt-config/ConfigFile.h
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "utils/EncryptionUtils.h"
+#include "utils/OptionalUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+class ConfigLine {
+public:
+ ConfigLine(std::string line);
+ ConfigLine(const std::string& key, const std::string& value);
+
+ void updateValue(const std::string& value);
+
+ std::string line_;
+ std::string key_;
+ std::string value_;
+};
+
+inline bool operator==(const ConfigLine& left, const ConfigLine& right) {
+ return left.line_ == right.line_;
+}
+
+class ConfigFile {
+public:
+ ConfigFile(const std::string& file_path);
+
+ utils::optional<std::string> getValue(const std::string& key) const;
+ void update(const std::string& key, const std::string& value);
+ void insertAfter(const std::string& after_key, const std::string& key, const std::string& value);
+ void append(const std::string& key, const std::string& value);
+ int erase(const std::string& key);
+
+ void writeTo(const std::string& file_path) const;
+
+ size_t size() const { return config_lines_.size(); }
+
+ int encryptSensitiveProperties(const utils::crypto::Bytes& encryption_key);
+
+private:
+ friend class ConfigFileTestAccessor;
+ friend bool operator==(const ConfigFile&, const ConfigFile&);
+ using Lines = std::vector<ConfigLine>;
+
+ Lines::const_iterator findKey(const std::string& key) const;
+ Lines::iterator findKey(const std::string& key);
+ std::vector<std::string> getSensitiveProperties() const;
+ static std::vector<std::string> mergeProperties(std::vector<std::string> properties,
+ const std::vector<std::string>& additional_properties);
+
+ Lines config_lines_;
+};
+
+inline bool operator==(const ConfigFile& left, const ConfigFile& right) {
+ return left.config_lines_ == right.config_lines_;
+}
Review comment:
Both equality comparisons should have an `operator!=` counterpart, preferably implemented in terms of `operator==`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org