[jira] [Created] (MINIFICPP-1501) Enforce that Processor::onTrigger calls are always invoked with a valid session

["Adam Hunyadi (Jira)" <ji...@apache.org>]

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)