mirror of https://github.com/qt/qtgrpc.git
QGrpcHttp2Channel: implement filterServerMetadata option
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 <alexey.edelev@qt.io>
This commit is contained in:
parent
25782e8866
commit
5bc854e901
|
|
@ -68,6 +68,8 @@ using namespace QtGrpc;
|
||||||
{serializationFormat}, or other options by constructing it with a
|
{serializationFormat}, or other options by constructing it with a
|
||||||
QGrpcChannelOptions containing the required customizations.
|
QGrpcChannelOptions containing the required customizations.
|
||||||
|
|
||||||
|
\note \l{QGrpcChannelOptions::filterServerMetadata} is enabled by default.
|
||||||
|
|
||||||
\section2 Transportation scheme
|
\section2 Transportation scheme
|
||||||
|
|
||||||
The QGrpcHttp2Channel implementation prefers different transportation
|
The QGrpcHttp2Channel implementation prefers different transportation
|
||||||
|
|
@ -312,6 +314,7 @@ public:
|
||||||
|
|
||||||
private:
|
private:
|
||||||
[[nodiscard]] HPack::HttpHeader constructInitialHeaders() const;
|
[[nodiscard]] HPack::HttpHeader constructInitialHeaders() const;
|
||||||
|
[[nodiscard]] bool constructFilterServerMetadata() const;
|
||||||
[[nodiscard]] QGrpcHttp2ChannelPrivate *channelPriv() const;
|
[[nodiscard]] QGrpcHttp2ChannelPrivate *channelPriv() const;
|
||||||
[[nodiscard]] QGrpcHttp2Channel *channel() const;
|
[[nodiscard]] QGrpcHttp2Channel *channel() const;
|
||||||
[[nodiscard]] bool handleContextExpired();
|
[[nodiscard]] bool handleContextExpired();
|
||||||
|
|
@ -324,6 +327,7 @@ private:
|
||||||
State m_state = State::Idle;
|
State m_state = State::Idle;
|
||||||
const bool m_endStreamAtFirstData;
|
const bool m_endStreamAtFirstData;
|
||||||
bool m_writesDoneSent = false;
|
bool m_writesDoneSent = false;
|
||||||
|
bool m_filterServerMetadata;
|
||||||
QTimer m_deadlineTimer;
|
QTimer m_deadlineTimer;
|
||||||
|
|
||||||
Q_DISABLE_COPY_MOVE(Http2Handler)
|
Q_DISABLE_COPY_MOVE(Http2Handler)
|
||||||
|
|
@ -434,7 +438,7 @@ private:
|
||||||
Http2Handler::Http2Handler(QGrpcOperationContext *operation, QGrpcHttp2ChannelPrivate *parent,
|
Http2Handler::Http2Handler(QGrpcOperationContext *operation, QGrpcHttp2ChannelPrivate *parent,
|
||||||
bool endStream)
|
bool endStream)
|
||||||
: QObject(parent), m_operation(operation), m_initialHeaders(constructInitialHeaders()),
|
: 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
|
// 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;
|
// can no longer perform any meaningful work. We allow it to be deleted;
|
||||||
|
|
@ -598,6 +602,13 @@ HPack::HttpHeader Http2Handler::constructInitialHeaders() const
|
||||||
return headers;
|
return headers;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool Http2Handler::constructFilterServerMetadata() const
|
||||||
|
{
|
||||||
|
return m_operation->callOptions()
|
||||||
|
.filterServerMetadata()
|
||||||
|
.value_or(channel()->channelOptions().filterServerMetadata().value_or(true));
|
||||||
|
}
|
||||||
|
|
||||||
QGrpcHttp2ChannelPrivate *Http2Handler::channelPriv() const
|
QGrpcHttp2ChannelPrivate *Http2Handler::channelPriv() const
|
||||||
{
|
{
|
||||||
return qobject_cast<QGrpcHttp2ChannelPrivate *>(this->parent());
|
return qobject_cast<QGrpcHttp2ChannelPrivate *>(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 "
|
qGrpcWarning("Received unexcpected gRPC-reserved header: { key: %s, value: %s } in "
|
||||||
"phase: %s",
|
"phase: %s",
|
||||||
k.data(), v.data(), QDebug::toBytes(phase).data());
|
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) {
|
if (validation.requireHttpStatus && !validation.hasHttpStatus) {
|
||||||
|
|
|
||||||
|
|
@ -47,6 +47,7 @@ private Q_SLOTS:
|
||||||
#if QT_DEPRECATED_SINCE(6, 13)
|
#if QT_DEPRECATED_SINCE(6, 13)
|
||||||
void deprecatedMetadata();
|
void deprecatedMetadata();
|
||||||
#endif
|
#endif
|
||||||
|
void metadata_data();
|
||||||
void metadata();
|
void metadata();
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -171,8 +172,17 @@ void QtGrpcClientUnaryCallTest::deprecatedMetadata()
|
||||||
|
|
||||||
#endif // QT_DEPRECATED_SINCE(6, 13)
|
#endif // QT_DEPRECATED_SINCE(6, 13)
|
||||||
|
|
||||||
|
void QtGrpcClientUnaryCallTest::metadata_data()
|
||||||
|
{
|
||||||
|
QTest::addColumn<bool>("filterServerMetadata");
|
||||||
|
QTest::addRow("filterServerMetadata(true)") << true;
|
||||||
|
QTest::addRow("filterServerMetadata(false)") << false;
|
||||||
|
}
|
||||||
|
|
||||||
void QtGrpcClientUnaryCallTest::metadata()
|
void QtGrpcClientUnaryCallTest::metadata()
|
||||||
{
|
{
|
||||||
|
// TODO: QTBUG-138407
|
||||||
|
QFETCH(const bool, filterServerMetadata);
|
||||||
QGrpcCallOptions opt;
|
QGrpcCallOptions opt;
|
||||||
QMultiHash<QByteArray, QByteArray> clientMd{
|
QMultiHash<QByteArray, QByteArray> clientMd{
|
||||||
{ "request_initial", "3" },
|
{ "request_initial", "3" },
|
||||||
|
|
@ -180,6 +190,7 @@ void QtGrpcClientUnaryCallTest::metadata()
|
||||||
{ "request_sum", "20" },
|
{ "request_sum", "20" },
|
||||||
{ "request_sum", "10" }
|
{ "request_sum", "10" }
|
||||||
};
|
};
|
||||||
|
opt.setFilterServerMetadata(filterServerMetadata);
|
||||||
opt.setMetadata(clientMd);
|
opt.setMetadata(clientMd);
|
||||||
auto reply = client()->testMetadata({}, opt);
|
auto reply = client()->testMetadata({}, opt);
|
||||||
QSignalSpy replyFinishedSpy(reply.get(), &QGrpcCallReply::finished);
|
QSignalSpy replyFinishedSpy(reply.get(), &QGrpcCallReply::finished);
|
||||||
|
|
@ -191,6 +202,10 @@ void QtGrpcClientUnaryCallTest::metadata()
|
||||||
QVERIFY(args.first().value<QGrpcStatus>().isOk());
|
QVERIFY(args.first().value<QGrpcStatus>().isOk());
|
||||||
|
|
||||||
const auto &initialMd = reply->serverInitialMetadata();
|
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");
|
auto initialIt = initialMd.equal_range("response_initial");
|
||||||
QCOMPARE_EQ(std::distance(initialIt.first, initialIt.second), 3);
|
QCOMPARE_EQ(std::distance(initialIt.first, initialIt.second), 3);
|
||||||
QCOMPARE_EQ(initialIt.first.value(), "2");
|
QCOMPARE_EQ(initialIt.first.value(), "2");
|
||||||
|
|
@ -200,6 +215,10 @@ void QtGrpcClientUnaryCallTest::metadata()
|
||||||
QCOMPARE_EQ(initialIt.first.value(), "0");
|
QCOMPARE_EQ(initialIt.first.value(), "0");
|
||||||
|
|
||||||
const auto &trailingMd = reply->serverTrailingMetadata();
|
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");
|
auto trailingIt = trailingMd.equal_range("response_trailing");
|
||||||
QCOMPARE_EQ(std::distance(trailingIt.first, trailingIt.second), 2);
|
QCOMPARE_EQ(std::distance(trailingIt.first, trailingIt.second), 2);
|
||||||
QCOMPARE_EQ(trailingIt.first.value(), "1");
|
QCOMPARE_EQ(trailingIt.first.value(), "1");
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue