Skip to content

Commit f6bbcab

Browse files
author
ivan-skryabin
committed
fix grpc: trim whitespaces in status message
The gRPC specification requires status-messages to be percent-encoded but allows spaces (0x20) to remain unencoded. However, this conflicts with HTTP/2 RFC9113 which strictly forbids leading and trailing whitespace in header field values. This fix trims whitespaces from gRPC status messages before transmission to ensure compliance with HTTP/2 commit_hash:02486c5f7dbcf56c31e146b041e1a603c7f88bbb
1 parent 1692b18 commit f6bbcab

File tree

14 files changed

+420
-3
lines changed

14 files changed

+420
-3
lines changed

‎.mapping.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2141,6 +2141,15 @@
21412141
"grpc/functional_tests/basic_server/unused_proto/CMakeLists.txt":"taxi/uservices/userver/grpc/functional_tests/basic_server/unused_proto/CMakeLists.txt",
21422142
"grpc/functional_tests/basic_server/unused_proto/proto/samples/unused.proto":"taxi/uservices/userver/grpc/functional_tests/basic_server/unused_proto/proto/samples/unused.proto",
21432143
"grpc/functional_tests/basic_server/unused_proto/proto/ya.make":"taxi/uservices/userver/grpc/functional_tests/basic_server/unused_proto/proto/ya.make",
2144+
"grpc/functional_tests/error_status/CMakeLists.txt":"taxi/uservices/userver/grpc/functional_tests/error_status/CMakeLists.txt",
2145+
"grpc/functional_tests/error_status/config_vars.yaml":"taxi/uservices/userver/grpc/functional_tests/error_status/config_vars.yaml",
2146+
"grpc/functional_tests/error_status/main.cpp":"taxi/uservices/userver/grpc/functional_tests/error_status/main.cpp",
2147+
"grpc/functional_tests/error_status/proto/functional_tests/error_status.proto":"taxi/uservices/userver/grpc/functional_tests/error_status/proto/functional_tests/error_status.proto",
2148+
"grpc/functional_tests/error_status/src/error_status_server.cpp":"taxi/uservices/userver/grpc/functional_tests/error_status/src/error_status_server.cpp",
2149+
"grpc/functional_tests/error_status/src/error_status_server.hpp":"taxi/uservices/userver/grpc/functional_tests/error_status/src/error_status_server.hpp",
2150+
"grpc/functional_tests/error_status/static_config.yaml":"taxi/uservices/userver/grpc/functional_tests/error_status/static_config.yaml",
2151+
"grpc/functional_tests/error_status/tests/conftest.py":"taxi/uservices/userver/grpc/functional_tests/error_status/tests/conftest.py",
2152+
"grpc/functional_tests/error_status/tests/test_error_status.py":"taxi/uservices/userver/grpc/functional_tests/error_status/tests/test_error_status.py",
21442153
"grpc/functional_tests/lowlevel/CMakeLists.txt":"taxi/uservices/userver/grpc/functional_tests/lowlevel/CMakeLists.txt",
21452154
"grpc/functional_tests/lowlevel/main.cpp":"taxi/uservices/userver/grpc/functional_tests/lowlevel/main.cpp",
21462155
"grpc/functional_tests/lowlevel/proto/samples/greeter.proto":"taxi/uservices/userver/grpc/functional_tests/lowlevel/proto/samples/greeter.proto",
@@ -2323,6 +2332,7 @@
23232332
"grpc/include/userver/ugrpc/server/impl/service_internals.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/server/impl/service_internals.hpp",
23242333
"grpc/include/userver/ugrpc/server/impl/service_worker.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/server/impl/service_worker.hpp",
23252334
"grpc/include/userver/ugrpc/server/impl/service_worker_impl.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/server/impl/service_worker_impl.hpp",
2335+
"grpc/include/userver/ugrpc/server/impl/status_utils.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/server/impl/status_utils.hpp",
23262336
"grpc/include/userver/ugrpc/server/impl/stream_adapter.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/server/impl/stream_adapter.hpp",
23272337
"grpc/include/userver/ugrpc/server/metadata_utils.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/server/metadata_utils.hpp",
23282338
"grpc/include/userver/ugrpc/server/middlewares/access_log/component.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/server/middlewares/access_log/component.hpp",
@@ -2461,6 +2471,7 @@
24612471
"grpc/src/ugrpc/server/impl/service_defaults.hpp":"taxi/uservices/userver/grpc/src/ugrpc/server/impl/service_defaults.hpp",
24622472
"grpc/src/ugrpc/server/impl/service_worker.cpp":"taxi/uservices/userver/grpc/src/ugrpc/server/impl/service_worker.cpp",
24632473
"grpc/src/ugrpc/server/impl/service_worker_impl.cpp":"taxi/uservices/userver/grpc/src/ugrpc/server/impl/service_worker_impl.cpp",
2474+
"grpc/src/ugrpc/server/impl/status_utils.cpp":"taxi/uservices/userver/grpc/src/ugrpc/server/impl/status_utils.cpp",
24642475
"grpc/src/ugrpc/server/middlewares/access_log/component.cpp":"taxi/uservices/userver/grpc/src/ugrpc/server/middlewares/access_log/component.cpp",
24652476
"grpc/src/ugrpc/server/middlewares/access_log/log_extra.cpp":"taxi/uservices/userver/grpc/src/ugrpc/server/middlewares/access_log/log_extra.cpp",
24662477
"grpc/src/ugrpc/server/middlewares/access_log/middleware.cpp":"taxi/uservices/userver/grpc/src/ugrpc/server/middlewares/access_log/middleware.cpp",

‎grpc/functional_tests/CMakeLists.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,6 @@ add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}-middleware-client)
1919

2020
add_subdirectory(lowlevel)
2121
add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}-lowlevel)
22+
23+
add_subdirectory(error_status)
24+
add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}-error-status)

‎grpc/functional_tests/error_status/CMakeLists.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
project(userver-grpc-tests-error-status CXX)
2+
3+
file(GLOB SOURCES src/*.*pp)
4+
5+
add_executable(${PROJECT_NAME} main.cpp ${SOURCES})
6+
7+
include(UserverGrpcTargets)
8+
userver_add_grpc_library(${PROJECT_NAME}-proto PROTOS functional_tests/error_status.proto)
9+
target_link_libraries(${PROJECT_NAME} ${PROJECT_NAME}-proto)
10+
11+
target_include_directories(${PROJECT_NAME} PRIVATE src)
12+
13+
userver_chaos_testsuite_add(TESTS_DIRECTORY tests)

‎grpc/functional_tests/error_status/config_vars.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# yaml
2+
server-name: test-core-metrics 1.0
3+
service-name: test_core_metrics
4+
logger-level: info
5+
6+
config-server-url: http://localhost:8083/
7+
server-port: 8185
8+
monitor-server-port: 8186
9+
10+
testsuite-enabled: false
11+
12+
userver-dumps-root: /var/cache/test_core_metrics/userver-dumps/
13+
access-log-path: /var/log/test_core_metrics/access.log
14+
access-tskv-log-path: /var/log/test_core_metrics/access_tskv.log
15+
default-log-path: /var/log/test_core_metrics/server.log
16+
secdist-path: /etc/test_core_metrics/secure_data.json
17+
18+
config-cache: /var/cache/test_core_metrics/config_cache.json
19+
20+
greeter-client-endpoint: $grpc_mockserver

‎grpc/functional_tests/error_status/main.cpp‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#include <userver/components/minimal_server_component_list.hpp>
2+
#include <userver/congestion_control/component.hpp>
3+
#include <userver/testsuite/testsuite_support.hpp>
4+
#include <userver/ugrpc/server/component_list.hpp>
5+
#include <userver/utils/daemon_run.hpp>
6+
7+
#include <error_status_server.hpp>
8+
9+
int main(int argc, const char* const argv[]) {
10+
const auto component_list = components::MinimalServerComponentList()
11+
.Append<components::TestsuiteSupport>()
12+
.Append<congestion_control::Component>()
13+
.AppendComponentList(ugrpc::server::MinimalComponentList())
14+
.Append<functional_tests::ErrorStatusServiceComponent>();
15+
16+
return utils::DaemonMain(argc, argv, component_list);
17+
}

‎grpc/functional_tests/error_status/proto/functional_tests/error_status.proto‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
syntax = "proto3";
2+
3+
package functional_tests.api;
4+
5+
service ErrorStatusService {
6+
rpc ReturnErrorStatus(ErrorStatusRequest) returns(ErrorStatusResponse) {}
7+
8+
rpc ThrowErrorWithStatusException(ErrorStatusRequest) returns(ErrorStatusResponse) {}
9+
rpc ThrowRuntimeErrorException(RuntimeErrorRequest) returns(ErrorStatusResponse) {}
10+
11+
rpc ReturnStreamErrorStatus(ErrorStatusRequest) returns(stream ErrorStatusResponse) {}
12+
rpc ThrowStreamErrorWithStatusException(ErrorStatusRequest) returns(stream ErrorStatusResponse) {}
13+
}
14+
15+
message ErrorStatusRequest {
16+
string status_code = 1;
17+
string status_message = 2;
18+
}
19+
20+
message RuntimeErrorRequest {
21+
string message = 1;
22+
}
23+
24+
message ErrorStatusResponse {
25+
}

‎grpc/functional_tests/error_status/src/error_status_server.cpp‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
#include <error_status_server.hpp>
2+
3+
namespace functional_tests {
4+
5+
static constexpr int kCountSend = 3;
6+
7+
ErrorStatusServiceComponent::ReturnErrorStatusResult
8+
ErrorStatusServiceComponent::ReturnErrorStatus(CallContext&, api::ErrorStatusRequest&& request) {
9+
const grpc::StatusCode request_status_code = ugrpc::StatusCodeFromString(request.status_code());
10+
if (request_status_code == grpc::StatusCode::OK) {
11+
return api::ErrorStatusResponse{};
12+
}
13+
return grpc::Status{
14+
request_status_code,
15+
request.status_message(),
16+
"",
17+
};
18+
}
19+
20+
ErrorStatusServiceComponent::ThrowErrorWithStatusExceptionResult
21+
ErrorStatusServiceComponent::ThrowErrorWithStatusException(CallContext&, api::ErrorStatusRequest&& request) {
22+
const grpc::StatusCode request_status_code = ugrpc::StatusCodeFromString(request.status_code());
23+
if (request_status_code == grpc::StatusCode::OK) {
24+
return api::ErrorStatusResponse{};
25+
}
26+
throw ugrpc::server::ErrorWithStatus{
27+
request_status_code,
28+
request.status_message(),
29+
};
30+
}
31+
32+
ErrorStatusServiceComponent::ThrowRuntimeErrorExceptionResult
33+
ErrorStatusServiceComponent::ThrowRuntimeErrorException(CallContext&, api::RuntimeErrorRequest&& request) {
34+
throw std::runtime_error(request.message());
35+
}
36+
37+
ErrorStatusServiceComponent::ReturnStreamErrorStatusResult ErrorStatusServiceComponent::ReturnStreamErrorStatus(
38+
CallContext&,
39+
api::ErrorStatusRequest&& request,
40+
ReturnStreamErrorStatusWriter& writer
41+
) {
42+
api::ErrorStatusResponse response;
43+
for (int i = 0; i < kCountSend; ++i) {
44+
writer.Write(response);
45+
}
46+
47+
const grpc::StatusCode request_status_code = ugrpc::StatusCodeFromString(request.status_code());
48+
if (request_status_code == grpc::StatusCode::OK) {
49+
return grpc::Status::OK;
50+
}
51+
52+
return grpc::Status{
53+
request_status_code,
54+
request.status_message(),
55+
"",
56+
};
57+
}
58+
59+
ErrorStatusServiceComponent::ThrowStreamErrorWithStatusExceptionResult
60+
ErrorStatusServiceComponent::ThrowStreamErrorWithStatusException(
61+
CallContext&,
62+
api::ErrorStatusRequest&& request,
63+
ThrowStreamErrorWithStatusExceptionWriter& writer
64+
) {
65+
api::ErrorStatusResponse response;
66+
for (int i = 0; i < kCountSend; ++i) {
67+
writer.Write(response);
68+
}
69+
70+
const grpc::StatusCode request_status_code = ugrpc::StatusCodeFromString(request.status_code());
71+
if (request_status_code == grpc::StatusCode::OK) {
72+
return grpc::Status::OK;
73+
}
74+
75+
throw ugrpc::server::ErrorWithStatus{
76+
request_status_code,
77+
request.status_message(),
78+
};
79+
}
80+
81+
yaml_config::Schema ErrorStatusServiceComponent::GetStaticConfigSchema() {
82+
return yaml_config::MergeSchemas<ugrpc::server::ServiceComponentBase>(R"(
83+
type: object
84+
description: gRPC error status service component for testing error handling and status codes
85+
additionalProperties: false
86+
properties: {}
87+
)");
88+
}
89+
90+
} // namespace functional_tests

‎grpc/functional_tests/error_status/src/error_status_server.hpp‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#pragma once
2+
3+
#include <grpc/status.h>
4+
5+
#include <userver/components/component.hpp>
6+
#include <userver/ugrpc/server/exceptions.hpp>
7+
#include <userver/ugrpc/status_codes.hpp>
8+
#include <userver/utest/using_namespace_userver.hpp>
9+
#include <userver/yaml_config/merge_schemas.hpp>
10+
11+
#include <functional_tests/error_status_service.usrv.pb.hpp>
12+
13+
namespace functional_tests {
14+
15+
class ErrorStatusServiceComponent final : public api::ErrorStatusServiceBase::Component {
16+
public:
17+
static constexpr std::string_view kName = "error-status-service";
18+
19+
ErrorStatusServiceComponent(const components::ComponentConfig& config, const components::ComponentContext& context)
20+
: api::ErrorStatusServiceBase::Component(config, context) {}
21+
22+
ReturnErrorStatusResult ReturnErrorStatus(CallContext& context, api::ErrorStatusRequest&& request) override;
23+
24+
ThrowErrorWithStatusExceptionResult
25+
ThrowErrorWithStatusException(CallContext& context, api::ErrorStatusRequest&& request) override;
26+
27+
ThrowRuntimeErrorExceptionResult
28+
ThrowRuntimeErrorException(CallContext& context, api::RuntimeErrorRequest&& request) override;
29+
30+
ReturnStreamErrorStatusResult ReturnStreamErrorStatus(
31+
CallContext& context,
32+
api::ErrorStatusRequest&& request,
33+
ReturnStreamErrorStatusWriter& writer
34+
) override;
35+
36+
ThrowStreamErrorWithStatusExceptionResult ThrowStreamErrorWithStatusException(
37+
CallContext& context,
38+
api::ErrorStatusRequest&& request,
39+
ThrowStreamErrorWithStatusExceptionWriter& writer
40+
) override;
41+
42+
static yaml_config::Schema GetStaticConfigSchema();
43+
};
44+
45+
} // namespace functional_tests

‎grpc/functional_tests/error_status/static_config.yaml‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# yaml
2+
components_manager:
3+
components:
4+
# The required common components
5+
logging:
6+
fs-task-processor: fs-task-processor
7+
loggers:
8+
default:
9+
file_path: '@stderr'
10+
level: debug
11+
overflow_behavior: discard
12+
13+
# gRPC server and service
14+
grpc-server:
15+
port: 8091
16+
17+
congestion-control:
18+
19+
grpc-server-logging:
20+
msg-size-log-limit: 10
21+
22+
grpc-server-middlewares-pipeline:
23+
24+
error-status-service:
25+
task-processor: main-task-processor
26+
27+
# http server
28+
server:
29+
listener:
30+
port: 8092
31+
task_processor: main-task-processor
32+
listener-monitor:
33+
port: $monitor-server-port
34+
port#fallback: 8086
35+
connection:
36+
in_buffer_size: 32768
37+
requests_queue_size_threshold: 100
38+
task_processor: main-task-processor
39+
40+
testsuite-support:
41+
42+
default_task_processor: main-task-processor
43+
44+
task_processors:
45+
grpc-blocking-task-processor: # For blocking gRPC channel creation
46+
thread_name: grpc-worker
47+
worker_threads: 2
48+
main-task-processor: # For non-blocking operations
49+
worker_threads: 4
50+
fs-task-processor: # For blocking filesystem operations
51+
worker_threads: 2

‎grpc/functional_tests/error_status/tests/conftest.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import functional_tests.error_status_pb2_grpc as error_status_services
2+
import pytest
3+
4+
pytest_plugins = ['pytest_userver.plugins.grpc']
5+
6+
7+
@pytest.fixture
8+
def greeter_mock(grpc_mockserver):
9+
return grpc_mockserver.mock_factory(error_status_services.ErrorStatusServiceServicer)
10+
11+
12+
@pytest.fixture
13+
def grpc_client(grpc_channel):
14+
return error_status_services.ErrorStatusServiceStub(grpc_channel)

0 commit comments

Comments
 (0)