From 5bc854e901b10c8b028a0bb0f6c1171ebefba770 Mon Sep 17 00:00:00 2001 From: Dennis Oberst Date: Tue, 8 Jul 2025 15:49:47 +0200 Subject: [PATCH] QGrpcHttp2Channel: implement filterServerMetadata option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement the filtering of internal and reserved keys for the server metadata. gRPC over HTTP2 responses are well defined and such information shouldn't be provided to users. At least not by default. Ref: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses This patch changes the filtering to be applied by default. [ChangeLog][QGrpcHttp2Channel/QGrpcOperation][Important Behavior Changes] QGrpcOperation::serverInitialMetadata() and QGrpcOperation::serverTrailingMetadata() no longer include any internal gRPC or HTTP/2 pseudo‑headers by default. Fixes: QTBUG-138363 Change-Id: I4af9e8abe60799e817f47faa5de4c2d0e41854be Reviewed-by: Alexey Edelev --- src/grpc/qgrpchttp2channel.cpp | 19 +++++++++++++++++-- .../unarycall/tst_grpc_client_unarycall.cpp | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/grpc/qgrpchttp2channel.cpp b/src/grpc/qgrpchttp2channel.cpp index 2cdb9136..462813cc 100644 --- a/src/grpc/qgrpchttp2channel.cpp +++ b/src/grpc/qgrpchttp2channel.cpp @@ -68,6 +68,8 @@ using namespace QtGrpc; {serializationFormat}, or other options by constructing it with a QGrpcChannelOptions containing the required customizations. + \note \l{QGrpcChannelOptions::filterServerMetadata} is enabled by default. + \section2 Transportation scheme The QGrpcHttp2Channel implementation prefers different transportation @@ -312,6 +314,7 @@ public: private: [[nodiscard]] HPack::HttpHeader constructInitialHeaders() const; + [[nodiscard]] bool constructFilterServerMetadata() const; [[nodiscard]] QGrpcHttp2ChannelPrivate *channelPriv() const; [[nodiscard]] QGrpcHttp2Channel *channel() const; [[nodiscard]] bool handleContextExpired(); @@ -324,6 +327,7 @@ private: State m_state = State::Idle; const bool m_endStreamAtFirstData; bool m_writesDoneSent = false; + bool m_filterServerMetadata; QTimer m_deadlineTimer; Q_DISABLE_COPY_MOVE(Http2Handler) @@ -434,7 +438,7 @@ private: Http2Handler::Http2Handler(QGrpcOperationContext *operation, QGrpcHttp2ChannelPrivate *parent, bool endStream) : QObject(parent), m_operation(operation), m_initialHeaders(constructInitialHeaders()), - m_endStreamAtFirstData(endStream) + m_endStreamAtFirstData(endStream), m_filterServerMetadata(constructFilterServerMetadata()) { // If the context (lifetime bound to the user) is destroyed, this handler // can no longer perform any meaningful work. We allow it to be deleted; @@ -598,6 +602,13 @@ HPack::HttpHeader Http2Handler::constructInitialHeaders() const return headers; } +bool Http2Handler::constructFilterServerMetadata() const +{ + return m_operation->callOptions() + .filterServerMetadata() + .value_or(channel()->channelOptions().filterServerMetadata().value_or(true)); +} + QGrpcHttp2ChannelPrivate *Http2Handler::channelPriv() const { return qobject_cast(this->parent()); @@ -815,9 +826,13 @@ void Http2Handler::handleHeaders(const HPack::HttpHeader &headers, HeaderPhase p qGrpcWarning("Received unexcpected gRPC-reserved header: { key: %s, value: %s } in " "phase: %s", k.data(), v.data(), QDebug::toBytes(phase).data()); + } else { // Custom-Metadata + metadata.insert(k, v); + continue; } - metadata.insert(k, v); + if (!m_filterServerMetadata) + metadata.insert(k, v); } if (validation.requireHttpStatus && !validation.hasHttpStatus) { diff --git a/tests/auto/grpc/client/unarycall/tst_grpc_client_unarycall.cpp b/tests/auto/grpc/client/unarycall/tst_grpc_client_unarycall.cpp index b50af773..c843db47 100644 --- a/tests/auto/grpc/client/unarycall/tst_grpc_client_unarycall.cpp +++ b/tests/auto/grpc/client/unarycall/tst_grpc_client_unarycall.cpp @@ -47,6 +47,7 @@ private Q_SLOTS: #if QT_DEPRECATED_SINCE(6, 13) void deprecatedMetadata(); #endif + void metadata_data(); void metadata(); }; @@ -171,8 +172,17 @@ void QtGrpcClientUnaryCallTest::deprecatedMetadata() #endif // QT_DEPRECATED_SINCE(6, 13) +void QtGrpcClientUnaryCallTest::metadata_data() +{ + QTest::addColumn("filterServerMetadata"); + QTest::addRow("filterServerMetadata(true)") << true; + QTest::addRow("filterServerMetadata(false)") << false; +} + void QtGrpcClientUnaryCallTest::metadata() { + // TODO: QTBUG-138407 + QFETCH(const bool, filterServerMetadata); QGrpcCallOptions opt; QMultiHash clientMd{ { "request_initial", "3" }, @@ -180,6 +190,7 @@ void QtGrpcClientUnaryCallTest::metadata() { "request_sum", "20" }, { "request_sum", "10" } }; + opt.setFilterServerMetadata(filterServerMetadata); opt.setMetadata(clientMd); auto reply = client()->testMetadata({}, opt); QSignalSpy replyFinishedSpy(reply.get(), &QGrpcCallReply::finished); @@ -191,6 +202,10 @@ void QtGrpcClientUnaryCallTest::metadata() QVERIFY(args.first().value().isOk()); const auto &initialMd = reply->serverInitialMetadata(); + if (filterServerMetadata) + QCOMPARE_EQ(initialMd.size(), 3); + else + QCOMPARE_GT(initialMd.size(), 3); auto initialIt = initialMd.equal_range("response_initial"); QCOMPARE_EQ(std::distance(initialIt.first, initialIt.second), 3); QCOMPARE_EQ(initialIt.first.value(), "2"); @@ -200,6 +215,10 @@ void QtGrpcClientUnaryCallTest::metadata() QCOMPARE_EQ(initialIt.first.value(), "0"); const auto &trailingMd = reply->serverTrailingMetadata(); + if (filterServerMetadata) + QCOMPARE_EQ(trailingMd.size(), 3); + else + QCOMPARE_GT(trailingMd.size(), 3); auto trailingIt = trailingMd.equal_range("response_trailing"); QCOMPARE_EQ(std::distance(trailingIt.first, trailingIt.second), 2); QCOMPARE_EQ(trailingIt.first.value(), "1");