Details
-
Task
-
Status: Resolved
-
Minor
-
Resolution: Duplicate
-
0.7.0
-
None
Description
Background:
Processor::onTrigger(..., core::ProcessSession*) calls take sessions by pointers even though they are never expected to be called with a nullptr argument. We could check whether this ptr is null in the onTrigger calls, and use it dereferenced, but this would produce a lot of redundant code. Changing the onTrigger signature would be a serious API break, so we should not change it.
Currently session is not checked for validity before derefencing:
➜ git --no-pager grep 'session->' | grep -i 'processors/' | cut -d ":" -f1 | less | sort | uniq | xargs -n1 sh -c 'echo $1 && egrep "if (.*session.*)" $1' sh extensions/aws/processors/DeleteS3Object.cpp extensions/aws/processors/FetchS3Object.cpp extensions/aws/processors/PutS3Object.cpp extensions/civetweb/processors/ListenHTTP.cpp extensions/http-curl/processors/InvokeHTTP.cpp extensions/mqtt/processors/ConsumeMQTT.cpp extensions/mqtt/processors/ConvertHeartBeat.cpp extensions/mqtt/processors/ConvertJSONAck.cpp extensions/mqtt/processors/PublishMQTT.cpp extensions/openwsman/processors/SourceInitiatedSubscriptionListener.cpp extensions/sftp/processors/FetchSFTP.cpp extensions/sftp/processors/ListSFTP.cpp if (createAndTransferFlowFileFromChild(session, hostname, port, username, file)) { if (!createAndTransferFlowFileFromChild(session, hostname, port, username, updated_entity)) { extensions/sftp/processors/PutSFTP.cpp if (!this->processOne(context, session)) { extensions/standard-processors/processors/AppendHostInfo.cpp extensions/standard-processors/processors/ExecuteProcess.cpp extensions/standard-processors/processors/ExtractText.cpp extensions/standard-processors/processors/GenerateFlowFile.cpp extensions/standard-processors/processors/GetFile.cpp extensions/standard-processors/processors/GetTCP.cpp extensions/standard-processors/processors/HashContent.cpp extensions/standard-processors/processors/ListenSyslog.cpp extensions/standard-processors/processors/LogAttribute.cpp extensions/standard-processors/processors/PutFile.cpp extensions/standard-processors/processors/RetryFlowFile.cpp extensions/standard-processors/processors/RouteOnAttribute.cpp extensions/standard-processors/processors/TailFile.cpp if (!session->existsFlowFileInRelationship(Success)) { extensions/standard-processors/processors/UpdateAttribute.cpp extensions/standard-processors/tests/unit/GetTCPTests.cpp extensions/standard-processors/tests/unit/ProcessorTests.cpp
Proposal:
If we want to have this added safety of checking pointers before deferencing them, we can enforce adding a gsl_Expects(session) to all of our implementations that overload the mentioned signature by adding a static code-check next to the linter check that enforces the presence at the start of the function using this overload. We can also provide another overload that takes session by reference - and by default the original implementation would do the checks and call into this one, so overloads on this would not require additional checks. This would mean checking for nullptr in Processor::onTrigger(shrd_ptr<>, shrd_ptr<>) and Processor::onTrigger(*, *) and forwarding the call to Processor::onTrigger(&, &) and moving overloads to the (&, &) version whenever possible.
Attachments
Issue Links
- is duplicated by
-
MINIFICPP-1415 Pass references to onTrigger and onSchedule instead of shared_ptr
- Resolved