Adam Hunyadi created MINIFICPP-1501:
---------------------------------------
Summary: Enforce that Processor::onTrigger calls are always
invoked with a valid session
Key: MINIFICPP-1501
URL: https://issues.apache.org/jira/browse/MINIFICPP-1501
Project: Apache NiFi MiNiFi C++
Issue Type: Task
Affects Versions: 0.7.0
Reporter: Adam Hunyadi
Assignee: Adam Hunyadi
Fix For: 1.0.0
*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:
{code:bash|title=Session dereferencing}
➜ 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
{code}
*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.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
