szaszm commented on a change in pull request #848:
URL: https://github.com/apache/nifi-minifi-cpp/pull/848#discussion_r465624329
##########
File path: libminifi/src/sitetosite/SiteToSiteClient.cpp
##########
@@ -499,8 +499,9 @@ int16_t SiteToSiteClient::send(std::string transactionID,
DataPacket *packet, co
return -1;
}
- ret = transaction->getStream().writeData(reinterpret_cast<uint8_t
*>(const_cast<char*>(packet->payload_.c_str())), len);
- if (ret != (int64_t)len) {
+ ret = transaction->getStream().writeData(reinterpret_cast<uint8_t
*>(const_cast<char*>(packet->payload_.c_str())),
+ gsl::narrow<int>(len));
+ if (ret != static_cast<int64_t>(len)) {
Review comment:
uint64_t to int64_t is a narrowing conversion
##########
File path: extensions/http-curl/processors/InvokeHTTP.cpp
##########
@@ -337,7 +337,7 @@ void InvokeHTTP::onTrigger(const
std::shared_ptr<core::ProcessContext> &context,
const std::vector<char> &response_body = client.getResponseBody();
const std::vector<std::string> &response_headers = client.getHeaders();
- int64_t http_code = client.getResponseCode();
+ int http_code = gsl::narrow<int>(client.getResponseCode());
Review comment:
I would check the response code before assuming that it's valid. I'm not
sure whether curl validates the number before returning, but I wouldn't want a
malicious server or MITM to be able to crash (`std::terminate` on narrowing
failure) the minifi c++ agent.
##########
File path: nanofi/src/core/cstream.c
##########
@@ -153,9 +148,9 @@ int readUTFLen(uint32_t * utflen, cstream * stream) {
return ret;
}
-int readUTF(char * buf, uint64_t buflen, cstream * stream) {
+int readUTF(char * buf, uint32_t buflen, cstream * stream) {
Review comment:
Why uint32_t? Why not e.g. size_t or int?
##########
File path: extensions/expression-language/Expression.cpp
##########
@@ -194,9 +194,12 @@ Value expr_toLower(const std::vector<Value> &args) {
Value expr_substring(const std::vector<Value> &args) {
if (args.size() < 3) {
- return Value(args[0].asString().substr(args[1].asUnsignedLong()));
+ size_t offset = gsl::narrow<size_t>(args[1].asUnsignedLong());
+ return Value(args[0].asString().substr(offset));
} else {
- return Value(args[0].asString().substr(args[1].asUnsignedLong(),
args[2].asUnsignedLong()));
+ size_t offset = gsl::narrow<size_t>(args[1].asUnsignedLong());
+ size_t count = gsl::narrow<size_t>(args[2].asUnsignedLong());
+ return Value(args[0].asString().substr(offset, count));
Review comment:
:+1:
##########
File path: libminifi/include/io/CRCStream.h
##########
@@ -224,7 +222,7 @@ template<typename T>
int CRCStream<T>::readData(uint8_t *buf, int buflen) {
int ret = child_stream_->read(buf, buflen);
if (ret > 0) {
- crc_ = crc32(crc_, buf, ret);
+ crc_ = crc32(gsl::narrow<uLong>(crc_), buf, ret);
Review comment:
I suggest changing the type of `crc_` to `uint32_t` and `static_assert`
that `uLong` has the same width and signedness as `uint32_t`. This should make
`gsl::narrow` calls redundant.
----------------------------------------------------------------
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:
us...@infra.apache.org
