Segmentation fault occurs in CLI - CoAP Example when using block-wise. about openthread HOT 6 CLOSED

I may have fixed this issue.
Here is diff:

ubuntu@xxxxx:~/workspace/openSource/openthread$ git diff
diff --git a/src/core/coap/coap.cpp b/src/core/coap/coap.cpp
index 696d0fc83..64e148792 100644
--- a/src/core/coap/coap.cpp
+++ b/src/core/coap/coap.cpp
@@ -655,7 +655,7 @@ Error CoapBase::PrepareNextBlockRequest(Message::BlockType aType,
         }
 
         // Copy option
-        SuccessOrExit(error = iterator.ReadOptionValue(&optionBuf));
+        SuccessOrExit(error = iterator.ReadOptionValue(optionBuf));
         SuccessOrExit(error = aRequest.AppendOption(optionNumber, iterator.GetOption()->GetLength(), &optionBuf));
     }

ReadOptionValue accepts a void* or &uint64_t type，and optionBuf is a uint64_t type. Obviously, this is a typo.

from openthread.

@eeumay , thanks for raising this issue and working to resolve it. Would you like to submit a PR with the fix?

from openthread.

@eeumay , thanks for raising this issue and working to resolve it. Would you like to submit a PR with the fix?

I'm on it. I will submit a PR and I'm waiting for GitHub Actions. But for copying option correctly, optionBuf should be uint8_t array. I'm not sure how to define a fixed langth adapting to an uncertain length due to the theoretical max is too big, and I decide to use the max length of the uri path which is necessary. Do you have any advice?

from openthread.

I'm on it. I will submit a PR and I'm waiting for GitHub Actions. But for copying option correctly, optionBuf should be uint8_t array. I'm not sure how to define a fixed langth adapting to an uncertain length due to the theoretical max is too big, and I decide to use the max length of the uri path which is necessary. Do you have any advice?

@eeumay , good observation. I believe the length issue was present in the original contribution:

• #4247

@abtink , who last updated the ReadOptionValue() method, any thoughts on how to address?

from openthread.

Using a fixed (max size) array can work but would be limited.

A more flexible (but more complex) approach can be to add a new AppendOption() flavor that directly copies the option value from a passed Option::Iterator. It can then copy the bytes from one message to another using Message::AppendBytesFromMessage(), eliminating the need for an intermediate buffer.

from openthread.

@jwhui @abtink Thanks for your advice.
I submit a PR to fix copying option.#9894
But it still failed that Coap parsed the previous old request (in CoapBase::PrepareNextBlockRequest()), because CoapBase::FindRelatedRequest() did return a Message appending metaData, which cannot be directly iterated. In this case, Option::Iterator::Read() would reach metaData and probably return kErrorParse.
I will open another issue to talk about it.

from openthread.