OpenAPITools / OpenAPITools/openapi-generator

[REQ][C++][Pistache] Reconsider default error handling in generated code.

Open
#9,909 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Enhancement: Feature
Dominant language
Java
Stars
26.8k
Forks
7.7k
PR merge metrics
PR metrics pending

Description

Is your feature request related to a problem? Please describe.

In 8f5639554e96a9beaf0b4c87b9dac52afcc1ef83 (PR #9251), I myself have introduced a more advanced error handling for the pistache server.

In the handleParsingException function and the handleOperationException function an unhandled exception will be matched to a pair with an http return code and an error message that will be sent to the client.
These functions can be overriden by the API impl if custom behaviour is necessary, but the default implementation (see code snippet below) will send out the what() message of any unhandled exception.

std::pair<Pistache::Http::Code, std::string> {{classname}}::handleParsingException(const std::exception& ex) const noexcept
{
    try {
        throw;
    } catch (nlohmann::detail::exception &e) {
        return std::make_pair(Pistache::Http::Code::Bad_Request, e.what());
    } catch ({{helpersNamespace}}::ValidationException &e) {
        return std::make_pair(Pistache::Http::Code::Bad_Request, e.what());
    } catch (std::exception &e) {
        return std::make_pair(Pistache::Http::Code::Internal_Server_Error, e.what())
    }
}
std::pair<Pistache::Http::Code, std::string> {{classname}}::handleOperationException(const std::exception& ex) const noexcept
{
    return std::make_pair(Pistache::Http::Code::Internal_Server_Error, ex.what());
}

For the use case I have, this solution is perfect, but I did not consider the consequences for other use cases.
Returning the error message for any unhandled exception, might leak sensitive information to the client, which could be damaging for publicly available APIs.

Describe the solution you'd like

I believe the decision I made should be reconsidered, and the default behaviour of the generated code should try to limit the information sent to the client.
The handleParsingException should only catch exceptions inside generated code, so it should be fairly controlled.
But I belive that only the json exception and ValidationException can be left the same, as json deserialization and validation exceptions apply to the data the client has sent to the server.

Describe alternatives you've considered

None - I merely want to start a discussion here.
I'm also tagging @muttleyxd and @etherealjoy as you two are part of the C++ technical committee.

Additional context

None

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with modules/openapi-generator/src/main/resources/cpp-pistache-server/api-source.mustache, especially handleParsingException and handleOperationException. Review the current default handling and the security implications of returning exception messages, then establish the intended safe behavior while preserving client-data errors such as JSON and ValidationException cases. Done means the default generated server behavior has an agreed, tested information-disclosure policy.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api, backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.