Previously the helper returned early in case no service section was
found in the proto file. If we supply multiple files, this function
should simply skip those. Otherwise it's not possible to process such a
list variable.
Pick-to: 6.10 6.9 6.8
Change-Id: Iab31fa81404f2df2ec4ad628810ff7f948b3eb5f
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
For understanding the flow better, if needed. Change the log for failed
cancellations to be a warning. This information is usefull to see
without special logging rules!
Pick-to: 6.10 6.9 6.8
Change-Id: I5e30962381c4f476d8e9e1258f5af10c7586942c
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Currently, if TLS or QLocalSocket is explicitly requested but
unavailable, we fall back to TCP. This is not a safe approach.
This patch asserts on that condition and uses qCFatal to indicate the
mismatch instead of silently falling back to another transport.
Given the security aspects of this file, this strengthens guarantees by
ensuring the requested transport is actually used.
Also: Make construction initialization friendly!
Separated concerns by splitting the large constructor into smaller
segments, achieving the same result through helper functions in the
initializer list.
Added a SocketType enum to clearly identify the transport method. Since
m_isLocalSocket was already removed, this provides more detail without
increasing storage.
[ChangeLog][QGrpcHttp2Channel][Important Behavior Changes]
Requesting TLS or QLocalSocket now fails with a fatal error if the
requested transport is unavailable.
Fixes: QTBUG-139597
Pick-to: 6.10 6.9 6.8
Change-Id: I2914d8a5764436d08b4502aa7d8cf6c4fdeb60d0
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
-A list of basic security topics when implementing gRPC
-Add links to other modules and relevant pages.
Task-number: QTBUG-138812
Pick-to: 6.9 6.10
Change-Id: I523f07cb516641771eb1457091f381a2e09b0026
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
As part of reviewing the application examples and following a structure, this
change covers:
-Language changes
-Follows the strucutre
-Includes \sa link to All Qt Examples
Fixes: QTBUG-137964
Pick-to: 6.10 6.9 6.8
Change-Id: Ic14d7042e6db277c1584b69ba46f7748f53642ab
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
Remove the warning that incorrectly triggers when a second HTTP/2
SETTINGS frame is received.
According to RFC, Section 6.5, a SETTINGS frame "MAY be sent at any
other time by either endpoint over the lifetime of the connection."
Ref: https://www.rfc-editor.org/rfc/rfc7540#section-6.5
Pick-to: 6.10 6.9 6.8
Change-Id: I83b0238405801a853c20f22629b12f67edce26e2
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
It's the single point where a RPC finishes and the correct place to stop
the timer.
Pick-to: 6.10 6.9 6.8
Change-Id: I2965a938da32a8d78ccfb48f7f5ad37f63ee4363
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
It's not used anywhere in the code
Pick-to: 6.10 6.9 6.8
Change-Id: I388800f73f67f9f37f008590c9f190aa091d9c6b
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Instead of connecting each and every Http2Handler to the socket error,
use QGrpcHttp2ChannelPrivate::handleSocketError to iterate over
alive Http2Handler and send errors right from this handler. This saves
some memory and time on handling errorOccurred signal.
Pick-to: 6.10 6.9 6.8
Change-Id: I907e24425aafe3dccca19100d02fe7adffb1fdaa
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
Fixes a compiler warning from implicit conversion by introducing a safe
qsizetype cast. Also improves readability and efficiency by renaming
variables and pre-allocating list storage aswell as adding a missing
move.
Pick-to: 6.10 6.9 6.8
Change-Id: I5eb34621690893e06e68c7b821e93e3462388595
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Repeated enum fields should not ignore the 'packed' protobuf attribute,
so if 'packed' is set to 'false', FieldFlag::NonPacked should land to
the message field attributes, indicating to serializer the expected
output format.
Fixes: QTBUG-138683
Pick-to: 6.8 6.9 6.10
Change-Id: I1c2d2bee32b42f60310307e1f3638ba2b3c236a3
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
The \generatelist command takes an optional argument
that affect how the members are listed, for example,
`classesbymodule` which creates an annotated list of
the classes in a named C++ module.
`groupsbymodule` is not recognized or documented. QDoc
falls back to generating a list of all members of a
group in this case. While the end result is correct,
this relies on an undocumented feature that is likely
to change and break.
Replace these instances with an explicit \annotatedlist
command that produces the same output.
Pick-to: 6.10
Task-number: QTBUG-138901
Change-Id: Iffa5720e53177d1b996d49f9a607e2a1fa91bb0a
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
Removed redundant 'QObject::' namespace to streamline code and enhance
readability, especially in lambda handlers.
Due to our formatting, all lambda handlers will be unnecessarily aligned
to the very right side. Given that connections and lambda handlers are
very common in this codebase this improves the readability of those
lambda handlers.
Pick-to: 6.10 6.9 6.8
Change-Id: I178a838c7702382b4b3845c729d7c11eeeb1c8d1
Reviewed-by: Tatiana Borisova <tatiana.borisova@qt.io>
Easily allows to have a deeper inspection of the system by adding more
debug prints, which can be enabled with the logging category.
Pick-to: 6.10 6.9 6.8
Change-Id: I86d7f6c0c53c412a79a3d66f515fa1cd6757a023
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Given the complexity of the http2channel implementation it comes natural
to extend its logging clarity. This aligns with best practices in Qt
development. Also this makes it easier to debug the system remotely.
Pick-to: 6.10 6.9 6.8
Change-Id: Idf00020408b678fc6b17a25db538abd5a838bced
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
1. Remove nesting by reversing the state check.
2. Split the messageReceived calculation to be outside of the function
call. Given that this is an important calculation it should be easily
understandable.
Pick-to: 6.10 6.9 6.8
Change-Id: Icfaa29a5dc92eb5b1c86469639a3deb90b5aacd0
Reviewed-by: Tatiana Borisova <tatiana.borisova@qt.io>
Re-order the argument to the Http2Handler ctor. The parent should come
first. Furthermore rename the m_operation to m_context as I think
context is a more fitting name for this important member.
This improves the readability and makes it easier to follow the code.
Pick-to: 6.10 6.9 6.8
Change-Id: I396e205ec345d80a8cf2cfebe43625f72d39ac6e
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
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 option enables to control the filtering of the received server
metadata. Both options provide it as optional<bool>, so that
QAbstractGrpcChannel implementations should choose a reasonable default.
This has not been provided to the QQml*Options, as there is currently no
available API to access the server metadata.
[ChangeLog][QGrpc{Call,Channel}Options] Added the filterServerMetadata
property.
Task-number: QTBUG-138363
Change-Id: I325a3b4f6e68d63f0828a6deb1a7be883247614b
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Before, we did a lazy initialization in the deprecated noexcept getters
to provide a way of not storing the metadata twice. There is just no way
to preserve correctness whilst providing this option.
Whilst const-ref return values provide the most efficient way to access
them, it also most tightly constrains the implementation. Let that be a
lesson ...
We also remove the free-standing 'operator==' as we're comparing with
the matching containers now, there is no need for them (They should also
not be provided like this from QtGrpc, even though private).
Amends: 778371b8ea.
Pick-to: 6.10
Change-Id: I05c4a1f7d2eab00f41ebcad54d9a096b2dcdf540
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Rename the free function to 'toHash', as no merging was happening, this
name was misleading.
Improve the implementation by adding the missing 'reserve()' and simply
try_emplace() the pair into the hash. This will still take the newest
value as before but without constructing a temporary key list and having
the extra lookup with value().
Pick-to: 6.10
Change-Id: I4834feea43e20614caea3d587055c024012e0596
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
We forgot to deprecate the server-metadata interfaces when we deprecated
the client-metadata. The same problems apply here too!
Found during the 6.10 API-review.
[ChangeLog][Deprecation Notice]
Deprecate the metadata()/serverMetadata()/setServerMetadata() methods on
QGrpcOperation and QGrpcOperationContext that use QHash in favor of the
new server{Initial,Trailing}Metadata interfaces, that use QMultiHash and
provide the correct handling of the received metadata in their
respective phase. This is a behavior change as the old metadata()
interface now only returns the initial metadata.
Fixes: QTBUG-138039
Pick-to: 6.10
Change-Id: I307ee81fc353a0f4316fea2d10f56bb6910ae859
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Currently, the QGrpcOperation was finishing itself so that the channel
implementation should only cancel the corresponding RPC. This is the
only place such self-finishing is used.
Let the channel implementation rather take care of the cancellation
logic and emit the finished signal. The actual handler implementation is
responsible for itself, i.e. finished meaning it's done and no
communication will happen through this stream.
Simplify the logic for timeouts and requested cancellations.
Update the deadline testcase to not check for specific messages but
rather for non-empty messages.
Changes will only be relevant for custom channel implementation.
[ChangeLog][QGrpcOperationContext][Important Behavior Changes]
Cancellation logic should also emit finished now. Custom
QAbstractGrpcChannel implementations should adapt their logic.
Pick-to: 6.10 6.9 6.8
Change-Id: Ic4e70b50afe46b5a883f099a6cf245ea9a0e66c1
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Our lifetime management was extremely flawed, as there were many points
where Http2Handler instances were not properly deleted, resulting in
inactive zombie handlers that would never be cleaned up.
In a running event loop, there should be no active or pending handlers
left at the time of channel destruction. If we had previously asserted
this condition:
QGrpcHttp2ChannelPrivate::~QGrpcHttp2ChannelPrivate()
{
Q_ASSERT(children().isEmpty());
}
and then ran our tests, many cases would reveal that we were effectively
leaking memory due to inproper lifetime management on our side.
This is fixed by applying the following:
When the finished signal is emitted, the corresponding Http2Handler
should be deleted. It no longer makes sense to keep it alive beyond that
point, as this aligns with our documented lifetime for client-side RPC
handlers.
Transform the operation context into a QPointer to not steady convert
into shared_ptr's. Connect to the destroyed signal to keep track when
the user-side handler gets deleted.
We definitely don't want to take part in sharing the ownership (and
therefore lifetime) of the operationContext. Pass down a pointer very
early on so that no mistakes happen in the future (as to take a copy of
the shared_ptr in a lambda).
This is streamlined by introducing the finish and asyncFinish functions.
Task-number: QTBUG-128338
Fixes: QTBUG-129160
Pick-to: 6.10 6.9 6.8
Change-Id: I8e17f7832659fb348e7d05aeabe164b16b6ff283
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
The `m_connection` check was not needed as there are two places where
this is called:
- inside createHttp2Connection() -> connection gets created, 100% valid
- inside processOperation(), where we need the m_connection check
anyways.
Remove the check + error and replace it with an assert.
Pick-to: 6.10 6.9 6.8
Change-Id: Ib1d8a16db65cadfecb242f54a27ab70bb08a78fd
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
createHttp2Stream is calling finished asynchronously. It must report the
outcome back immediately though, as processOperation() depends on it.
Pick-to: 6.10 6.9 6.8
Change-Id: Ic01fea69dba491c8f119eb3a28523878a8e214d7
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
The QGrpcOperation::cancel() logic is already emitting
QGrpcOperation::finished() for us but the previous cancellation logic
was not deleting the cancelled handler.
Fix this by deleting unconditionally. This is fine for cancelled
handlers. Furthermore transform the cancel() function to return void.
Cancellation should be used unconditionally.
Task-number: QTBUG-129160
Pick-to: 6.10 6.9 6.8
Change-Id: Ifad7230a7592dc5d7691379031356afe1f8f8dc3
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
The QHttp2Stream implementation doesn't provide any handling or
verification of the HTTP/2 spec. The gRPC over HTTP/2 protocol clearly
defines how Response-Headers, Trailers and Trailers-Only have to behave.
Currently we are missing crucial steps of validation, like:
- HTTP status to be 200
- Trailers contain gRPC status
- Valid gRPC content-type found
Ref: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses
Create the handleHeaders() function to correctly handle the protocol.
Correctly differentiate between Initial, Trailers and TrailersOnly now.
Fixes: QTBUG-138494
Pick-to: 6.10 6.9 6.8
Change-Id: I0d2aba0f123a408b43dc4eb0f2a045898b41bc96
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
We should start the deadline-timer when the actual call is beginning and
this is indicated when the initial headers have been sent and not the
stream that has been attached. As a drive-by mark the timer as
single-shot.
Pick-to: 6.10 6.9 6.8
Change-Id: I1eb58d143e4934a3c0770cd3ff24ed47972a5289
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
The deletion was invalid and could result in a double free. If
createHttp2Connection() encountered expired handlers, it deleted them
but did not remove them from the container. Later, when
settingsFrameReceived() iterated over the same container, it would
encounter the already-deleted handler again.
Harden our handler management by relying on QObject parent <> child
relationship instead of additionally tracking the children by our own.
The handler (child) was already doing that. The channel didn't actually
need any active or pending containers. It's enough to handle the
processing through QObject::children().
We have to add the Q_OBJECT macro now to fully support parent <> child
relationship.
Pick-to: 6.10 6.9 6.8
Change-Id: Iee81e98e826f6019fdf525b167faa33944ebadb2
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Since the Http2Handler is largely self-managed, having a clear and
accurate state representation is crucial.
Previously, some key states were missing—particularly those relevant to
the header phase. This made it harder to ensure correct behavior and
state transitions.
This change introduces the 'Idle' and 'RequestHeadersSent' states to
better reflect the handler’s lifecycle. With this we can enforce more
robust validation and ensure the handler behaves as expected.
Furthermore we change the onDone() handling logic (to only allow it to
be called once) to use a bool. Using state management for this is
impractical. It's not a real state of our Http2Handler. It's leaking out
the internal stream state.
Pick-to: 6.10 6.9 6.8
Change-Id: I30b5c935c4fe3aaafb66ed91e25562afca2f8804
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
This patch follows C++ best practices. Refactor the
'prepareInitialRequest' function to directly return the constructed
headers so that we can use them in the initializer list.
Ref: https://isocpp.org/wiki/faq/ctors#init-lists
Pick-to: 6.10 6.9 6.8
Change-Id: I095d9ecc3574b8ad1ed344d16701102d2c79df92
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
For consistency. No null checking is needed as the channel will always
destroy all children before destroying itself.
Pick-to: 6.10 6.9 6.8
Change-Id: I4638cd906dc6c66d2559048bd147216518655110
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Extends the testcase. The call should fail here with a non-OK status
code.
Pick-to: 6.10 6.9 6.8
Change-Id: I4b8f833f2cb147d9b301004c51dc2a3388876fb6
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Extend the testcase to cover both, initial and trailing metadata. This
also shows the current problems with having a QHash metadata and also
the missing separation between initial and trailing metadata.
Pick-to: 6.10 6.9 6.8
Change-Id: I880846ae06e8db338cdb3629dafdff430cab3edb
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
For later usage in server metadata deprecation.
Pick-to: 6.10
Change-Id: I973943063b1ba0b065bba48abd12a1be008ebed4
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
These functions are not callable. Declaring them const allows them
to match more calls (specifically, const rvalues, which before would
have given "no matching overload" instead of "deleted" errors).
Pick-to: 6.10
Change-Id: I3d074d561456cf98fccacabea68204c92b7a4b38
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
The variable is used unconditionally.
Amends c91d02c007
Pick-to: 6.10 6.9 6.8
Change-Id: I36910a7b40f16a5a13bf2839a2e6c27aec5e7b26
Reviewed-by: Dennis Oberst <dennis.oberst@qt.io>
We store the host protoc and generator paths from crosscompiling
environment when building generator tests. The test is supposed to
run on target meanwhile, which surely will lead to an issue if
paths to the tools on target platform differ. This particulary happens
on windows arm64 machine when we copy the cross-compiled tests.
Pick-to: 6.10 6.9 6.8
Change-Id: I85971da1b429383cae0f842306629355fc5833c6
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Dennis Oberst
Qt Submodule Update Bot
Jerome Pasion
Jaishree Vyas
Alexey Edelev
Topi Reinio
Kai Köhne