From a5a7e30ec5d63e6d689f5ecfbb6d83b1731b226a Mon Sep 17 00:00:00 2001 From: levlam Date: Tue, 6 Aug 2019 03:29:16 +0300 Subject: [PATCH] Return td_api::error for incorrect requests in JSON interface. GitOrigin-RevId: 1c3e3f6c2b7a9828930bdfc185f332f6190467d1 --- example/cpp/tdjson_example.cpp | 20 ++++++++++++- td/generate/scheme/td_api.tl | 2 +- td/generate/tl_json_converter.cpp | 2 +- td/telegram/ClientJson.cpp | 50 +++++++++++++++++-------------- td/telegram/Td.cpp | 25 +++++++++++----- td/telegram/Td.h | 13 ++++---- 6 files changed, 73 insertions(+), 39 deletions(-) diff --git a/example/cpp/tdjson_example.cpp b/example/cpp/tdjson_example.cpp index cffb1413..2e261375 100644 --- a/example/cpp/tdjson_example.cpp +++ b/example/cpp/tdjson_example.cpp @@ -18,11 +18,29 @@ int main() { void *client = td_json_client_create(); // somehow share the client with other threads, which will be able to send requests via td_json_client_send + const bool test_incorrect_queries = true; + if (test_incorrect_queries) { + td_json_client_execute(nullptr, "{\"@type\":\"setLogVerbosityLevel\", \"new_verbosity_level\":3}"); + td_json_client_execute(nullptr, ""); + td_json_client_execute(nullptr, "test"); + td_json_client_execute(nullptr, "\"test\""); + td_json_client_execute(nullptr, "{\"@type\":\"test\", \"@extra\":1}"); + + td_json_client_send(client, "{\"@type\":\"getFileMimeType\"}"); + td_json_client_send(client, "{\"@type\":\"getFileMimeType\", \"@extra\":1}"); + td_json_client_send(client, "{\"@type\":\"getFileMimeType\", \"@extra\":null}"); + td_json_client_send(client, "{\"@type\":\"test\"}"); + td_json_client_send(client, "[]"); + td_json_client_send(client, "{\"@type\":\"test\", \"@extra\":1}"); + td_json_client_send(client, "{\"@type\":\"sendMessage\", \"chat_id\":true, \"@extra\":1}"); + td_json_client_send(client, "test"); + } + const double WAIT_TIMEOUT = 10.0; // seconds while (true) { const char *result = td_json_client_receive(client, WAIT_TIMEOUT); if (result != nullptr) { - // parse the result as JSON object and process it as an incoming update or an answer to a previously sent request + // parse the result as a JSON object and process it as an incoming update or an answer to a previously sent request // if (result is UpdateAuthorizationState with authorizationStateClosed) { // break; diff --git a/td/generate/scheme/td_api.tl b/td/generate/scheme/td_api.tl index 84b12eb2..45e0b08f 100644 --- a/td/generate/scheme/td_api.tl +++ b/td/generate/scheme/td_api.tl @@ -4010,5 +4010,5 @@ testProxy server:string port:int32 type:ProxyType = Ok; testGetDifference = Ok; //@description Does nothing and ensures that the Update object is used; for testing only. This is an offline method. Can be called before authorization testUseUpdate = Update; -//@description Returns the specified error and ensures that the Error object is used; for testing only. This is an offline method. Can be called before authorization @error The error to be returned +//@description Returns the specified error and ensures that the Error object is used; for testing only. This is an offline method. Can be called before authorization. Can be called synchronously @error The error to be returned testReturnError error:error = Error; diff --git a/td/generate/tl_json_converter.cpp b/td/generate/tl_json_converter.cpp index 5910ade4..1a22ae2f 100644 --- a/td/generate/tl_json_converter.cpp +++ b/td/generate/tl_json_converter.cpp @@ -151,7 +151,7 @@ void gen_tl_constructor_from_string(StringBuilder &sb, Slice name, const Vec &ve sb << "\n };\n"; sb << " auto it = m.find(str);\n"; sb << " if (it == m.end()) {\n" - << " return Status::Error(\"Unknown class\");\n" + << " return Status::Error(PSLICE() << \"Unknown class \\\"\" << str << \"\\\"\");\n" << " }\n" << " return it->second;\n"; sb << "}\n"; diff --git a/td/telegram/ClientJson.cpp b/td/telegram/ClientJson.cpp index c3f0be60..33e541e2 100644 --- a/td/telegram/ClientJson.cpp +++ b/td/telegram/ClientJson.cpp @@ -22,11 +22,22 @@ namespace td { -static Result, string>> to_request(Slice request) { +static td_api::object_ptr get_return_error_function(Slice error_message) { + auto error = td_api::make_object(400, error_message.str()); + return td_api::make_object(std::move(error)); +} + +static std::pair, string> to_request(Slice request) { auto request_str = request.str(); - TRY_RESULT(json_value, json_decode(request_str)); + auto r_json_value = json_decode(request_str); + if (r_json_value.is_error()) { + return {get_return_error_function(PSLICE() + << "Failed to parse request as JSON object: " << r_json_value.error().message()), + string()}; + } + auto json_value = r_json_value.move_as_ok(); if (json_value.type() != JsonValue::Type::Object) { - return Status::Error("Expected an Object"); + return {get_return_error_function("Expected a JSON object"), string()}; } string extra; @@ -36,8 +47,13 @@ static Result, string>> to_reques } td_api::object_ptr func; - TRY_STATUS(from_json(func, json_value)); - return std::make_pair(std::move(func), extra); + auto status = from_json(func, json_value); + if (status.is_error()) { + return {get_return_error_function(PSLICE() + << "Failed to parse JSON object as TDLib request: " << status.error().message()), + std::move(extra)}; + } + return std::make_pair(std::move(func), std::move(extra)); } static std::string from_response(const td_api::Object &object, const string &extra) { @@ -62,18 +78,13 @@ static CSlice store_string(std::string str) { } void ClientJson::send(Slice request) { - auto r_request = to_request(request); - if (r_request.is_error()) { - LOG(ERROR) << "Failed to parse " << tag("request", format::escaped(request)) << " " << r_request.error(); - return; - } - + auto parsed_request = to_request(request); std::uint64_t extra_id = extra_id_.fetch_add(1, std::memory_order_relaxed); - if (!r_request.ok_ref().second.empty()) { + if (!parsed_request.second.empty()) { std::lock_guard guard(mutex_); - extra_[extra_id] = std::move(r_request.ok_ref().second); + extra_[extra_id] = std::move(parsed_request.second); } - client_.send(Client::Request{extra_id, std::move(r_request.ok_ref().first)}); + client_.send(Client::Request{extra_id, std::move(parsed_request.first)}); } CSlice ClientJson::receive(double timeout) { @@ -95,14 +106,9 @@ CSlice ClientJson::receive(double timeout) { } CSlice ClientJson::execute(Slice request) { - auto r_request = to_request(request); - if (r_request.is_error()) { - LOG(ERROR) << "Failed to parse " << tag("request", format::escaped(request)) << " " << r_request.error(); - return {}; - } - - return store_string(from_response(*Client::execute(Client::Request{0, std::move(r_request.ok_ref().first)}).object, - r_request.ok().second)); + auto parsed_request = to_request(request); + return store_string(from_response(*Client::execute(Client::Request{0, std::move(parsed_request.first)}).object, + parsed_request.second)); } } // namespace td diff --git a/td/telegram/Td.cpp b/td/telegram/Td.cpp index 46294ab9..0629f23b 100644 --- a/td/telegram/Td.cpp +++ b/td/telegram/Td.cpp @@ -3422,6 +3422,7 @@ bool Td::is_synchronous_request(int32 id) { case td_api::setLogTagVerbosityLevel::ID: case td_api::getLogTagVerbosityLevel::ID: case td_api::addLogMessage::ID: + case td_api::testReturnError::ID: return true; default: return false; @@ -3433,7 +3434,6 @@ bool Td::is_preinitialization_request(int32 id) { case td_api::getCurrentState::ID: case td_api::setAlarm::ID: case td_api::testUseUpdate::ID: - case td_api::testReturnError::ID: case td_api::testCallEmpty::ID: case td_api::testSquareInt::ID: case td_api::testCallString::ID: @@ -3632,6 +3632,7 @@ td_api::object_ptr Td::static_request(td_api::object_ptr Td::do_static_request(const td_api::addLogMes return td_api::make_object(); } +td_api::object_ptr Td::do_static_request(td_api::testReturnError &request) { + if (request.error_ == nullptr) { + return td_api::make_object(404, "Not Found"); + } + + return std::move(request.error_); +} + // test -void Td::on_request(uint64 id, td_api::testNetwork &request) { +void Td::on_request(uint64 id, const td_api::testNetwork &request) { create_handler(id)->send(); } @@ -7677,24 +7686,24 @@ void Td::on_request(uint64 id, td_api::testProxy &request) { CREATE_REQUEST(TestProxyRequest, r_proxy.move_as_ok()); } -void Td::on_request(uint64 id, td_api::testGetDifference &request) { +void Td::on_request(uint64 id, const td_api::testGetDifference &request) { updates_manager_->get_difference("testGetDifference"); send_closure(actor_id(this), &Td::send_result, id, make_tl_object()); } -void Td::on_request(uint64 id, td_api::testUseUpdate &request) { +void Td::on_request(uint64 id, const td_api::testUseUpdate &request) { send_closure(actor_id(this), &Td::send_result, id, nullptr); } -void Td::on_request(uint64 id, td_api::testReturnError &request) { - send_closure(actor_id(this), &Td::send_result, id, std::move(request.error_)); +void Td::on_request(uint64 id, const td_api::testReturnError &request) { + UNREACHABLE(); } -void Td::on_request(uint64 id, td_api::testCallEmpty &request) { +void Td::on_request(uint64 id, const td_api::testCallEmpty &request) { send_closure(actor_id(this), &Td::send_result, id, make_tl_object()); } -void Td::on_request(uint64 id, td_api::testSquareInt &request) { +void Td::on_request(uint64 id, const td_api::testSquareInt &request) { send_closure(actor_id(this), &Td::send_result, id, make_tl_object(request.x_ * request.x_)); } diff --git a/td/telegram/Td.h b/td/telegram/Td.h index f292eabf..d6ccd443 100644 --- a/td/telegram/Td.h +++ b/td/telegram/Td.h @@ -1023,13 +1023,13 @@ class Td final : public NetQueryCallback { void on_request(uint64 id, const td_api::addLogMessage &request); // test - void on_request(uint64 id, td_api::testNetwork &request); + void on_request(uint64 id, const td_api::testNetwork &request); void on_request(uint64 id, td_api::testProxy &request); - void on_request(uint64 id, td_api::testGetDifference &request); - void on_request(uint64 id, td_api::testUseUpdate &request); - void on_request(uint64 id, td_api::testReturnError &request); - void on_request(uint64 id, td_api::testCallEmpty &request); - void on_request(uint64 id, td_api::testSquareInt &request); + void on_request(uint64 id, const td_api::testGetDifference &request); + void on_request(uint64 id, const td_api::testUseUpdate &request); + void on_request(uint64 id, const td_api::testReturnError &request); + void on_request(uint64 id, const td_api::testCallEmpty &request); + void on_request(uint64 id, const td_api::testSquareInt &request); void on_request(uint64 id, td_api::testCallString &request); void on_request(uint64 id, td_api::testCallBytes &request); void on_request(uint64 id, td_api::testCallVectorInt &request); @@ -1058,6 +1058,7 @@ class Td final : public NetQueryCallback { static td_api::object_ptr do_static_request(const td_api::setLogTagVerbosityLevel &request); static td_api::object_ptr do_static_request(const td_api::getLogTagVerbosityLevel &request); static td_api::object_ptr do_static_request(const td_api::addLogMessage &request); + static td_api::object_ptr do_static_request(td_api::testReturnError &request); static DbKey as_db_key(string key); Status init(DbKey key) TD_WARN_UNUSED_RESULT;