szaszm commented on code in PR #2251:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2251#discussion_r3898749126


##########
libminifi/src/c2/ControllerSocketProtocol.cpp:
##########
@@ -389,7 +390,7 @@ void 
ControllerSocketProtocol::writeFlowStatusResponse(io::BaseStream &stream) {
 
 void ControllerSocketProtocol::handleDescribe(io::BaseStream &stream) {
   std::string what;
-  const auto size = stream.read(what);
+  const auto size = stream.read(what, io::LengthPrefixSize::_16BIT, 1_KiB);

Review Comment:
   not using the MAX_COMPONENT_NAME_LENGTH symbolic constant, because this is 
not a component name, even tho I picked the same limit.



##########
libminifi/src/provenance/Provenance.cpp:
##########
@@ -339,14 +341,16 @@ bool 
ProvenanceEventRecordImpl::deserialize(io::InputStream &input_stream) {
   for (uint32_t i = 0; i < numAttributes; i++) {
     std::string key;
     {
-      const auto ret = input_stream.read(key);
+      // clamp attribute name / value to 64k (the 16bit length prefix maximum)

Review Comment:
   do we need these explanatory comments?



##########
minifi-api/common/include/minifi-cpp/io/InputStream.h:
##########
@@ -40,11 +45,13 @@ class InputStream : public virtual Stream {
   virtual size_t read(std::span<std::byte> out_buffer) = 0;
 
   /**
-   * Read string from stream. Use isError (Stream.h) to check for errors.
+   * Read length prefixed string from stream. Use isError (Stream.h) to check 
for errors.
    * @param str reference string
+   * @param length_prefix_size The wideness of the length prefix, 16bit or 
32bit

Review Comment:
   TODO
   ```suggestion
      * @param length_prefix_size The bit width of the length prefix, 16bit or 
32bit
   ```



##########
core-framework/common/src/io/InputStream.cpp:
##########
@@ -35,7 +37,7 @@ size_t InputStream::read(bool &value) {
 
 size_t InputStream::read(utils::Identifier &value) {
   std::string uuidStr;
-  const auto ret = read(uuidStr);
+  const auto ret = read(uuidStr, LengthPrefixSize::_16BIT, 36 /* characters in 
a UUID string */);

Review Comment:
   I've only added this comment to this place, but there are identifier reads 
elsewhere too. Refactor them to call this read overload, or introduce a 
symbolic constant into utils::Identifier?



-- 
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]

Reply via email to