Skip to content

Commit

Permalink
Remove exception text from error sending (#1048)
Browse files Browse the repository at this point in the history
Fixes #1037
  • Loading branch information
kuznetsss authored Dec 13, 2023
1 parent dd35a7c commit b1dc277
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 15 deletions.
28 changes: 26 additions & 2 deletions src/web/RPCServerHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,38 @@

#pragma once

#include "data/BackendInterface.h"
#include "feed/SubscriptionManager.h"
#include "rpc/Errors.h"
#include "rpc/Factories.h"
#include "rpc/JS.h"
#include "rpc/RPCHelpers.h"
#include "rpc/common/impl/APIVersionParser.h"
#include "util/JsonUtils.h"
#include "util/Profiler.h"
#include "util/Taggable.h"
#include "util/config/Config.h"
#include "util/log/Logger.h"
#include "web/impl/ErrorHandling.h"
#include "web/interface/ConnectionBase.h"

#include <boost/asio/spawn.hpp>
#include <boost/beast/core/error.hpp>
#include <boost/json/array.hpp>
#include <boost/json/object.hpp>
#include <boost/json/parse.hpp>
#include <boost/json/serialize.hpp>
#include <boost/system/system_error.hpp>
#include <ripple/protocol/jss.h>

#include <chrono>
#include <exception>
#include <functional>
#include <memory>
#include <ratio>
#include <stdexcept>
#include <string>
#include <utility>

namespace web {

Expand Down Expand Up @@ -104,11 +126,13 @@ class RPCServerHandler {
} catch (boost::system::system_error const& ex) {
// system_error thrown when json parsing failed
rpcEngine_->notifyBadSyntax();
web::detail::ErrorHelper(connection).sendJsonParsingError(ex.what());
web::detail::ErrorHelper(connection).sendJsonParsingError();
LOG(log_.warn()) << "Error parsing JSON: " << ex.what() << ". For request: " << request;
} catch (std::invalid_argument const& ex) {
// thrown when json parses something that is not an object at top level
rpcEngine_->notifyBadSyntax();
web::detail::ErrorHelper(connection).sendJsonParsingError(ex.what());
LOG(log_.warn()) << "Invalid argument error: " << ex.what() << ". For request: " << request;
web::detail::ErrorHelper(connection).sendJsonParsingError();
} catch (std::exception const& ex) {
LOG(perfLog_.error()) << connection->tag() << "Caught exception: " << ex.what();
rpcEngine_->notifyInternalError();
Expand Down
18 changes: 11 additions & 7 deletions src/web/impl/ErrorHandling.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,18 @@
#include "util/Assert.h"
#include "web/interface/ConnectionBase.h"

#include <boost/beast/http.hpp>
#include <boost/json.hpp>
#include <boost/beast/http/status.hpp>
#include <boost/json/object.hpp>
#include <boost/json/serialize.hpp>
#include <fmt/core.h>
#include <ripple/protocol/ErrorCodes.h>

#include <memory>
#include <optional>
#include <string>
#include <string_view>
#include <utility>
#include <variant>

namespace web::detail {

Expand Down Expand Up @@ -127,15 +133,13 @@ class ErrorHelper {
}

void
sendJsonParsingError(std::string_view reason) const
sendJsonParsingError() const
{
if (connection_->upgraded) {
connection_->send(
boost::json::serialize(rpc::makeError(rpc::RippledError::rpcBAD_SYNTAX)), boost::beast::http::status::ok
);
connection_->send(boost::json::serialize(rpc::makeError(rpc::RippledError::rpcBAD_SYNTAX)));
} else {
connection_->send(
fmt::format("Unable to parse request: {}", reason), boost::beast::http::status::bad_request
fmt::format("Unable to parse JSON from the request"), boost::beast::http::status::bad_request
);
}
}
Expand Down
23 changes: 22 additions & 1 deletion src/web/impl/HttpBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,41 @@

#include "main/Build.h"
#include "rpc/Errors.h"
#include "util/Taggable.h"
#include "util/log/Logger.h"
#include "util/prometheus/Http.h"
#include "web/DOSGuard.h"
#include "web/impl/AdminVerificationStrategy.h"
#include "web/interface/Concepts.h"
#include "web/interface/ConnectionBase.h"

#include <boost/asio/error.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/ssl/error.hpp>
#include <boost/beast/core.hpp>
#include <boost/beast/core/error.hpp>
#include <boost/beast/core/flat_buffer.hpp>
#include <boost/beast/http.hpp>
#include <boost/beast/http/error.hpp>
#include <boost/beast/http/field.hpp>
#include <boost/beast/http/message.hpp>
#include <boost/beast/http/status.hpp>
#include <boost/beast/http/string_body.hpp>
#include <boost/beast/http/verb.hpp>
#include <boost/beast/ssl.hpp>
#include <boost/core/ignore_unused.hpp>
#include <boost/json.hpp>

#include <boost/json/array.hpp>
#include <boost/json/parse.hpp>
#include <boost/json/serialize.hpp>
#include <ripple/protocol/ErrorCodes.h>

#include <cstddef>
#include <exception>
#include <functional>
#include <memory>
#include <string>
#include <utility>

namespace web::detail {

Expand Down
31 changes: 27 additions & 4 deletions src/web/impl/WsBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,40 @@

#pragma once

#include "rpc/Errors.h"
#include "rpc/common/Types.h"
#include "util/Taggable.h"
#include "util/log/Logger.h"
#include "web/DOSGuard.h"
#include "web/interface/Concepts.h"
#include "web/interface/ConnectionBase.h"

#include <boost/asio/buffer.hpp>
#include <boost/asio/error.hpp>
#include <boost/beast/core.hpp>
#include <boost/beast/websocket.hpp>

#include <iostream>
#include <boost/beast/core/error.hpp>
#include <boost/beast/core/flat_buffer.hpp>
#include <boost/beast/core/role.hpp>
#include <boost/beast/http/field.hpp>
#include <boost/beast/http/message.hpp>
#include <boost/beast/http/status.hpp>
#include <boost/beast/http/string_body.hpp>
#include <boost/beast/version.hpp>
#include <boost/beast/websocket/rfc6455.hpp>
#include <boost/beast/websocket/stream_base.hpp>
#include <boost/core/ignore_unused.hpp>
#include <boost/json/array.hpp>
#include <boost/json/parse.hpp>
#include <boost/json/serialize.hpp>
#include <ripple/protocol/ErrorCodes.h>

#include <cstddef>
#include <exception>
#include <functional>
#include <memory>
#include <queue>
#include <string>
#include <utility>

namespace web::detail {

Expand Down Expand Up @@ -153,7 +176,7 @@ class WsBase : public ConnectionBase, public std::enable_shared_from_this<WsBase
* If the DOSGuard is triggered, the message will be modified to include a warning
*/
void
send(std::string&& msg, http::status = http::status::ok) override
send(std::string&& msg, http::status) override
{
if (!dosGuard_.get().add(clientIp, msg.size())) {
auto jsonResponse = boost::json::parse(msg).as_object();
Expand Down
2 changes: 1 addition & 1 deletion unittests/web/RPCServerHandlerTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ TEST_F(WebRPCServerHandlerTest, HTTPTooBusy)
TEST_F(WebRPCServerHandlerTest, HTTPRequestNotJson)
{
static auto constexpr request = "not json";
static auto constexpr responsePrefix = "Unable to parse request: syntax error";
static auto constexpr responsePrefix = "Unable to parse JSON from the request";

EXPECT_CALL(*rpcEngine, notifyBadSyntax).Times(1);

Expand Down

0 comments on commit b1dc277

Please sign in to comment.