Skip to content

Commit 6ef51f6

Browse files
authored
FetchScriptResults. Fix crash on parsing of incorrect operation id (ydb-platform#15327)
1 parent b3d3961 commit 6ef51f6

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed

ydb/core/grpc_services/query/rpc_fetch_script_results.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,18 @@ class TFetchScriptResultsRPC : public TRpcRequestActor<TFetchScriptResultsRPC, T
125125
}
126126

127127
bool GetExecutionIdFromRequest() {
128-
TMaybe<TString> executionId = NKqp::ScriptExecutionIdFromOperation(GetProtoRequest()->operation_id());
129-
if (!executionId) {
130-
Reply(Ydb::StatusIds::BAD_REQUEST, "Invalid operation id");
128+
try {
129+
TMaybe<TString> executionId = NKqp::ScriptExecutionIdFromOperation(GetProtoRequest()->operation_id());
130+
if (!executionId) {
131+
Reply(Ydb::StatusIds::BAD_REQUEST, "Invalid operation id");
132+
return false;
133+
}
134+
ExecutionId = *executionId;
135+
return true;
136+
} catch (const std::exception& ex) {
137+
Reply(Ydb::StatusIds::BAD_REQUEST, TStringBuilder() << "Invalid operation id: " << ex.what());
131138
return false;
132139
}
133-
ExecutionId = *executionId;
134-
return true;
135140
}
136141

137142
private:

ydb/core/kqp/ut/federated_query/generic_ut/kqp_generic_provider_ut.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
#include <ydb/library/yql/providers/generic/connector/libcpp/ut_helpers/connector_client_mock.h>
66
#include <ydb/library/yql/providers/generic/connector/libcpp/ut_helpers/database_resolver_mock.h>
77
#include <ydb/library/yql/providers/s3/actors/yql_s3_actors_factory_impl.h>
8+
#include <ydb/public/api/protos/ydb_query.pb.h>
9+
#include <ydb/public/api/grpc/ydb_query_v1.grpc.pb.h>
10+
#include <ydb/public/sdk/cpp/src/library/grpc/client/grpc_client_low.h>
811
#include <ydb-cpp-sdk/client/operation/operation.h>
912
#include <ydb-cpp-sdk/client/query/query.h>
1013
#include <ydb-cpp-sdk/client/types/status_codes.h>
@@ -471,5 +474,39 @@ namespace NKikimr::NKqp {
471474
Y_UNIT_TEST(YdbFilterPushdown) {
472475
TestFilterPushdown(EProviderType::Ydb);
473476
}
477+
478+
void TestFailsOnIncorrectScriptExecutionOperation(const TString& operationId, const TString& fetchToken) {
479+
auto clientMock = std::make_shared<TConnectorClientMock>();
480+
auto databaseAsyncResolverMock = MakeDatabaseAsyncResolver(EProviderType::Ydb);
481+
auto appConfig = CreateDefaultAppConfig();
482+
auto s3ActorsFactory = NYql::NDq::CreateS3ActorsFactory();
483+
auto kikimr = MakeKikimrRunner(false, clientMock, databaseAsyncResolverMock, appConfig, s3ActorsFactory);
484+
485+
// Create trash query
486+
NYdbGrpc::TGRpcClientLow clientLow;
487+
488+
std::shared_ptr<grpc::Channel> channel;
489+
channel = grpc::CreateChannel("localhost:" + ToString(kikimr->GetTestServer().GetGRpcServer().GetPort()), grpc::InsecureChannelCredentials());
490+
{
491+
std::unique_ptr<Ydb::Query::V1::QueryService::Stub> stub;
492+
stub = Ydb::Query::V1::QueryService::NewStub(channel);
493+
grpc::ClientContext context;
494+
Ydb::Query::FetchScriptResultsRequest request;
495+
request.set_operation_id(operationId);
496+
request.set_fetch_token(fetchToken);
497+
Ydb::Query::FetchScriptResultsResponse response;
498+
grpc::Status st = stub->FetchScriptResults(&context, request, &response);
499+
UNIT_ASSERT(st.ok());
500+
UNIT_ASSERT_EQUAL_C(response.status(), Ydb::StatusIds::BAD_REQUEST, response);
501+
}
502+
}
503+
504+
Y_UNIT_TEST(TestFailsOnIncorrectScriptExecutionOperationId) {
505+
TestFailsOnIncorrectScriptExecutionOperation("trash", "");
506+
}
507+
508+
Y_UNIT_TEST(TestFailsOnIncorrectScriptExecutionFetchToken) {
509+
TestFailsOnIncorrectScriptExecutionOperation("", "trash");
510+
}
474511
}
475512
}

0 commit comments

Comments
 (0)