gfe-relnote: In QUIC, replace member variables current* with on stack struct ReceivedPacketInfo in QuicDispatcher. Refactor only, not protected. PiperOrigin-RevId: 254801890 Change-Id: I762a32bcd53528a2d80f75c9c57dad176a9b6fb1
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index da18b03..8aaec09 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -198,7 +198,6 @@ delete_sessions_alarm_( alarm_factory_->CreateAlarm(new DeleteSessionsAlarm(this))), buffered_packets_(this, helper_->GetClock(), alarm_factory_.get()), - current_packet_(nullptr), version_manager_(version_manager), last_error_(QUIC_NO_ERROR), new_sessions_allowed_per_event_loop_(0u), @@ -226,21 +225,12 @@ << " bytes:" << std::endl << QuicTextUtils::HexDump( QuicStringPiece(packet.data(), packet.length())); - current_self_address_ = self_address; - current_peer_address_ = peer_address; - // GetClientAddress must be called after current_peer_address_ is set. - current_client_address_ = GetClientAddress(); - current_packet_ = &packet; - - PacketHeaderFormat form = GOOGLE_QUIC_PACKET; - bool version_flag = false; - QuicVersionLabel version_label = 0; - QuicConnectionId destination_connection_id = EmptyQuicConnectionId(); - QuicConnectionId source_connection_id = EmptyQuicConnectionId(); + ReceivedPacketInfo packet_info(self_address, peer_address, packet); std::string detailed_error; const QuicErrorCode error = QuicFramer::ProcessPacketDispatcher( - packet, expected_server_connection_id_length_, &form, &version_flag, - &version_label, &destination_connection_id, &source_connection_id, + packet, expected_server_connection_id_length_, &packet_info.form, + &packet_info.version_flag, &packet_info.version_label, + &packet_info.destination_connection_id, &packet_info.source_connection_id, &detailed_error); if (error != QUIC_NO_ERROR) { // Packet has framing error. @@ -248,28 +238,26 @@ QUIC_DLOG(ERROR) << detailed_error; return; } - ParsedQuicVersion version = ParseQuicVersionLabel(version_label); - if (destination_connection_id.length() != + packet_info.version = ParseQuicVersionLabel(packet_info.version_label); + if (packet_info.destination_connection_id.length() != expected_server_connection_id_length_ && !should_update_expected_server_connection_id_length_ && !QuicUtils::VariableLengthConnectionIdAllowedForVersion( - version.transport_version)) { + packet_info.version.transport_version)) { SetLastError(QUIC_INVALID_PACKET_HEADER); QUIC_DLOG(ERROR) << "Invalid Connection Id Length"; return; } if (should_update_expected_server_connection_id_length_) { - expected_server_connection_id_length_ = destination_connection_id.length(); + expected_server_connection_id_length_ = + packet_info.destination_connection_id.length(); } - if (MaybeDispatchPacket(form, version_flag, version_label, version, - destination_connection_id, source_connection_id)) { + if (MaybeDispatchPacket(packet_info)) { // Packet has been dropped or successfully dispatched, stop processing. return; } - ProcessHeader(form, version_flag, version, destination_connection_id); - // TODO(wub): Consider invalidate the current_* variables so processing of - // the next packet does not use them incorrectly. + ProcessHeader(&packet_info); } QuicConnectionId QuicDispatcher::MaybeReplaceServerConnectionId( @@ -296,22 +284,15 @@ } bool QuicDispatcher::MaybeDispatchPacket( - PacketHeaderFormat form, - bool version_flag, - QuicVersionLabel version_label, - quic::ParsedQuicVersion version, - QuicConnectionId destination_connection_id, - QuicConnectionId source_connection_id) { - current_server_connection_id_ = destination_connection_id; - + const ReceivedPacketInfo& packet_info) { // Port zero is only allowed for unidirectional UDP, so is disallowed by QUIC. // Given that we can't even send a reply rejecting the packet, just drop the // packet. - if (current_peer_address_.port() == 0) { + if (packet_info.peer_address.port() == 0) { return true; } - QuicConnectionId server_connection_id = destination_connection_id; + QuicConnectionId server_connection_id = packet_info.destination_connection_id; // The IETF spec requires the client to generate an initial server // connection ID that is at least 64 bits long. After that initial @@ -321,9 +302,9 @@ if (server_connection_id.length() < kQuicMinimumInitialConnectionIdLength && server_connection_id.length() < expected_server_connection_id_length_ && !allow_short_initial_server_connection_ids_) { - DCHECK(version_flag); + DCHECK(packet_info.version_flag); DCHECK(QuicUtils::VariableLengthConnectionIdAllowedForVersion( - version.transport_version)); + packet_info.version.transport_version)); QUIC_DLOG(INFO) << "Packet with short destination connection ID " << server_connection_id << " expected " << static_cast<int>(expected_server_connection_id_length_); @@ -333,15 +314,15 @@ QUIC_DLOG(INFO) << "Adding connection ID " << server_connection_id << " to time-wait list."; StatelesslyTerminateConnection( - server_connection_id, form, version_flag, version, - QUIC_HANDSHAKE_FAILED, "Reject connection", + server_connection_id, packet_info.form, packet_info.version_flag, + packet_info.version, QUIC_HANDSHAKE_FAILED, "Reject connection", quic::QuicTimeWaitListManager::SEND_STATELESS_RESET); DCHECK(time_wait_list_manager_->IsConnectionIdInTimeWait( server_connection_id)); time_wait_list_manager_->ProcessPacket( - current_self_address_, current_peer_address_, server_connection_id, - form, GetPerPacketContext()); + packet_info.self_address, packet_info.peer_address, + server_connection_id, packet_info.form, GetPerPacketContext()); buffered_packets_.DiscardPackets(server_connection_id); } else { @@ -356,26 +337,26 @@ auto it = session_map_.find(server_connection_id); if (it != session_map_.end()) { DCHECK(!buffered_packets_.HasBufferedPackets(server_connection_id)); - it->second->ProcessUdpPacket(current_self_address_, current_peer_address_, - *current_packet_); + it->second->ProcessUdpPacket(packet_info.self_address, + packet_info.peer_address, packet_info.packet); return true; } if (buffered_packets_.HasChloForConnection(server_connection_id)) { - BufferEarlyPacket(server_connection_id, form != GOOGLE_QUIC_PACKET, - version); + BufferEarlyPacket(packet_info); return true; } - if (OnFailedToDispatchPacket(destination_connection_id)) { + if (OnFailedToDispatchPacket(packet_info)) { return true; } if (time_wait_list_manager_->IsConnectionIdInTimeWait(server_connection_id)) { // This connection ID is already in time-wait state. time_wait_list_manager_->ProcessPacket( - current_self_address_, current_peer_address_, destination_connection_id, - form, GetPerPacketContext()); + packet_info.self_address, packet_info.peer_address, + packet_info.destination_connection_id, packet_info.form, + GetPerPacketContext()); return true; } @@ -383,20 +364,21 @@ // Unless the packet provides a version, assume that we can continue // processing using our preferred version. - if (version_flag) { - if (!IsSupportedVersion(version)) { - if (ShouldCreateSessionForUnknownVersion(version_label)) { + if (packet_info.version_flag) { + if (!IsSupportedVersion(packet_info.version)) { + if (ShouldCreateSessionForUnknownVersion(packet_info.version_label)) { return false; } if (!crypto_config()->validate_chlo_size() || - current_packet_->length() >= kMinPacketSizeForVersionNegotiation) { + packet_info.packet.length() >= kMinPacketSizeForVersionNegotiation) { // Since the version is not supported, send a version negotiation // packet and stop processing the current packet. - QuicConnectionId client_connection_id = source_connection_id; + QuicConnectionId client_connection_id = + packet_info.source_connection_id; time_wait_list_manager()->SendVersionNegotiationPacket( server_connection_id, client_connection_id, - form != GOOGLE_QUIC_PACKET, GetSupportedVersions(), - current_self_address_, current_peer_address_, + packet_info.form != GOOGLE_QUIC_PACKET, GetSupportedVersions(), + packet_info.self_address, packet_info.peer_address, GetPerPacketContext()); } return true; @@ -406,37 +388,32 @@ return false; } -void QuicDispatcher::ProcessHeader(PacketHeaderFormat form, - bool version_flag, - ParsedQuicVersion version, - QuicConnectionId destination_connection_id) { - QuicConnectionId server_connection_id = destination_connection_id; +void QuicDispatcher::ProcessHeader(ReceivedPacketInfo* packet_info) { + QuicConnectionId server_connection_id = + packet_info->destination_connection_id; // Packet's connection ID is unknown. Apply the validity checks. // TODO(wub): Determine the fate completely in ValidityChecks, then call // ProcessUnauthenticatedHeaderFate in one place. - QuicPacketFate fate = - ValidityChecks(version_flag, version, destination_connection_id); + QuicPacketFate fate = ValidityChecks(*packet_info); ChloAlpnExtractor alpn_extractor; switch (fate) { case kFateProcess: - if (version.handshake_protocol == PROTOCOL_TLS1_3) { + if (packet_info->version.handshake_protocol == PROTOCOL_TLS1_3) { // TODO(nharper): Support buffering non-ClientHello packets when using // TLS. - ProcessChlo(form, version); + ProcessChlo(/*alpn=*/"", packet_info); break; } if (GetQuicFlag(FLAGS_quic_allow_chlo_buffering) && - !ChloExtractor::Extract(*current_packet_, GetSupportedVersions(), + !ChloExtractor::Extract(packet_info->packet, GetSupportedVersions(), config_->create_session_tag_indicators(), &alpn_extractor, server_connection_id.length())) { // Buffer non-CHLO packets. - BufferEarlyPacket(server_connection_id, form != GOOGLE_QUIC_PACKET, - version); + BufferEarlyPacket(*packet_info); break; } - current_alpn_ = alpn_extractor.ConsumeAlpn(); - ProcessChlo(form, version); + ProcessChlo(alpn_extractor.ConsumeAlpn(), packet_info); break; case kFateTimeWait: // Add this connection_id to the time-wait state, to safely reject @@ -445,15 +422,15 @@ << " to time-wait list."; QUIC_CODE_COUNT(quic_reject_fate_time_wait); StatelesslyTerminateConnection( - server_connection_id, form, version_flag, version, - QUIC_HANDSHAKE_FAILED, "Reject connection", + server_connection_id, packet_info->form, packet_info->version_flag, + packet_info->version, QUIC_HANDSHAKE_FAILED, "Reject connection", quic::QuicTimeWaitListManager::SEND_STATELESS_RESET); DCHECK(time_wait_list_manager_->IsConnectionIdInTimeWait( server_connection_id)); time_wait_list_manager_->ProcessPacket( - current_self_address_, current_peer_address_, server_connection_id, - form, GetPerPacketContext()); + packet_info->self_address, packet_info->peer_address, + server_connection_id, packet_info->form, GetPerPacketContext()); buffered_packets_.DiscardPackets(server_connection_id); break; @@ -461,26 +438,20 @@ } QuicDispatcher::QuicPacketFate QuicDispatcher::ValidityChecks( - bool version_flag, - ParsedQuicVersion /*version*/, - QuicConnectionId destination_connection_id) { + const ReceivedPacketInfo& packet_info) { // To have all the checks work properly without tears, insert any new check // into the framework of this method in the section for checks that return the // check's fate value. The sections for checks must be ordered with the // highest priority fate first. - // Checks that return kFateDrop. - - // Checks that return kFateTimeWait. - // All packets within a connection sent by a client before receiving a // response from the server are required to have the version negotiation flag // set. Since this may be a client continuing a connection we lost track of // via server restart, send a rejection to fast-fail the connection. - if (!version_flag) { + if (!packet_info.version_flag) { QUIC_DLOG(INFO) << "Packet without version arrived for unknown connection ID " - << destination_connection_id; + << packet_info.destination_connection_id; return kFateTimeWait; } @@ -758,10 +729,9 @@ } bool QuicDispatcher::ShouldCreateOrBufferPacketForConnection( - QuicConnectionId server_connection_id, - bool /*ietf_quic*/) { + const ReceivedPacketInfo& packet_info) { QUIC_VLOG(1) << "Received packet from new connection " - << server_connection_id; + << packet_info.destination_connection_id; return true; } @@ -782,80 +752,83 @@ alarm_factory_.get()); } -void QuicDispatcher::BufferEarlyPacket(QuicConnectionId server_connection_id, - bool ietf_quic, - ParsedQuicVersion version) { - bool is_new_connection = - !buffered_packets_.HasBufferedPackets(server_connection_id); - if (is_new_connection && !ShouldCreateOrBufferPacketForConnection( - server_connection_id, ietf_quic)) { +void QuicDispatcher::BufferEarlyPacket(const ReceivedPacketInfo& packet_info) { + bool is_new_connection = !buffered_packets_.HasBufferedPackets( + packet_info.destination_connection_id); + if (is_new_connection && + !ShouldCreateOrBufferPacketForConnection(packet_info)) { return; } EnqueuePacketResult rs = buffered_packets_.EnqueuePacket( - server_connection_id, ietf_quic, *current_packet_, current_self_address_, - current_peer_address_, /*is_chlo=*/false, - /*alpn=*/"", version); + packet_info.destination_connection_id, + packet_info.form != GOOGLE_QUIC_PACKET, packet_info.packet, + packet_info.self_address, packet_info.peer_address, /*is_chlo=*/false, + /*alpn=*/"", packet_info.version); if (rs != EnqueuePacketResult::SUCCESS) { - OnBufferPacketFailure(rs, server_connection_id); + OnBufferPacketFailure(rs, packet_info.destination_connection_id); } } -void QuicDispatcher::ProcessChlo(PacketHeaderFormat form, - ParsedQuicVersion version) { +void QuicDispatcher::ProcessChlo(const std::string& alpn, + ReceivedPacketInfo* packet_info) { if (!accept_new_connections_) { // Don't any create new connection. QUIC_CODE_COUNT(quic_reject_stop_accepting_new_connections); StatelesslyTerminateConnection( - current_server_connection_id(), form, /*version_flag=*/true, version, - QUIC_HANDSHAKE_FAILED, "Stop accepting new connections", + packet_info->destination_connection_id, packet_info->form, + /*version_flag=*/true, packet_info->version, QUIC_HANDSHAKE_FAILED, + "Stop accepting new connections", quic::QuicTimeWaitListManager::SEND_STATELESS_RESET); // Time wait list will reject the packet correspondingly. time_wait_list_manager()->ProcessPacket( - current_self_address(), current_peer_address(), - current_server_connection_id(), form, GetPerPacketContext()); + packet_info->self_address, packet_info->peer_address, + packet_info->destination_connection_id, packet_info->form, + GetPerPacketContext()); return; } - if (!buffered_packets_.HasBufferedPackets(current_server_connection_id_) && - !ShouldCreateOrBufferPacketForConnection(current_server_connection_id_, - form != GOOGLE_QUIC_PACKET)) { + if (!buffered_packets_.HasBufferedPackets( + packet_info->destination_connection_id) && + !ShouldCreateOrBufferPacketForConnection(*packet_info)) { return; } if (GetQuicFlag(FLAGS_quic_allow_chlo_buffering) && new_sessions_allowed_per_event_loop_ <= 0) { // Can't create new session any more. Wait till next event loop. - QUIC_BUG_IF( - buffered_packets_.HasChloForConnection(current_server_connection_id_)); + QUIC_BUG_IF(buffered_packets_.HasChloForConnection( + packet_info->destination_connection_id)); EnqueuePacketResult rs = buffered_packets_.EnqueuePacket( - current_server_connection_id_, form != GOOGLE_QUIC_PACKET, - *current_packet_, current_self_address_, current_peer_address_, - /*is_chlo=*/true, current_alpn_, version); + packet_info->destination_connection_id, + packet_info->form != GOOGLE_QUIC_PACKET, packet_info->packet, + packet_info->self_address, packet_info->peer_address, + /*is_chlo=*/true, alpn, packet_info->version); if (rs != EnqueuePacketResult::SUCCESS) { - OnBufferPacketFailure(rs, current_server_connection_id_); + OnBufferPacketFailure(rs, packet_info->destination_connection_id); } return; } - QuicConnectionId original_connection_id = current_server_connection_id_; - current_server_connection_id_ = - MaybeReplaceServerConnectionId(current_server_connection_id_, version); + QuicConnectionId original_connection_id = + packet_info->destination_connection_id; + packet_info->destination_connection_id = MaybeReplaceServerConnectionId( + original_connection_id, packet_info->version); // Creates a new session and process all buffered packets for this connection. QuicSession* session = - CreateQuicSession(current_server_connection_id_, current_peer_address_, - current_alpn_, version); - if (original_connection_id != current_server_connection_id_) { + CreateQuicSession(packet_info->destination_connection_id, + packet_info->peer_address, alpn, packet_info->version); + if (original_connection_id != packet_info->destination_connection_id) { session->connection()->AddIncomingConnectionId(original_connection_id); } QUIC_DLOG(INFO) << "Created new session for " - << current_server_connection_id_; - session_map_.insert( - std::make_pair(current_server_connection_id_, QuicWrapUnique(session))); + << packet_info->destination_connection_id; + session_map_.insert(std::make_pair(packet_info->destination_connection_id, + QuicWrapUnique(session))); std::list<BufferedPacket> packets = - buffered_packets_.DeliverPackets(current_server_connection_id_) + buffered_packets_.DeliverPackets(packet_info->destination_connection_id) .buffered_packets; // Process CHLO at first. - session->ProcessUdpPacket(current_self_address_, current_peer_address_, - *current_packet_); + session->ProcessUdpPacket(packet_info->self_address, + packet_info->peer_address, packet_info->packet); // Deliver queued-up packets in the same order as they arrived. // Do this even when flag is off because there might be still some packets // buffered in the store before flag is turned off. @@ -863,10 +836,6 @@ --new_sessions_allowed_per_event_loop_; } -const QuicSocketAddress QuicDispatcher::GetClientAddress() const { - return current_peer_address_; -} - bool QuicDispatcher::ShouldDestroySessionAsynchronously() { return true; } @@ -876,7 +845,7 @@ } bool QuicDispatcher::OnFailedToDispatchPacket( - QuicConnectionId /*destination_connection_id*/) { + const ReceivedPacketInfo& /*packet_info*/) { return false; }
diff --git a/quic/core/quic_dispatcher.h b/quic/core/quic_dispatcher.h index 65f3d28..8dd2d99 100644 --- a/quic/core/quic_dispatcher.h +++ b/quic/core/quic_dispatcher.h
@@ -150,12 +150,7 @@ // Returns true if packet is dropped or successfully dispatched (e.g., // processed by existing session, processed by time wait list, etc.), // otherwise, returns false and the packet needs further processing. - virtual bool MaybeDispatchPacket(PacketHeaderFormat form, - bool version_flag, - QuicVersionLabel version_label, - quic::ParsedQuicVersion version, - QuicConnectionId destination_connection_id, - QuicConnectionId source_connection_id); + virtual bool MaybeDispatchPacket(const ReceivedPacketInfo& packet_info); // Values to be returned by ValidityChecks() to indicate what should be done // with a packet. @@ -169,32 +164,19 @@ // This method is called by ProcessHeader on packets not associated with a // known connection ID. It applies validity checks and returns a // QuicPacketFate to tell what should be done with the packet. - virtual QuicPacketFate ValidityChecks( - bool version_flag, - ParsedQuicVersion version, - QuicConnectionId destination_connection_id); + // TODO(fayang): Merge ValidityChecks into MaybeDispatchPacket. + virtual QuicPacketFate ValidityChecks(const ReceivedPacketInfo& packet_info); // Create and return the time wait list manager for this dispatcher, which // will be owned by the dispatcher as time_wait_list_manager_ virtual QuicTimeWaitListManager* CreateQuicTimeWaitListManager(); - // Called when |server_connection_id| doesn't have an open connection yet, - // to buffer |current_packet_| until it can be delivered to the connection. - void BufferEarlyPacket(QuicConnectionId server_connection_id, - bool ietf_quic, - ParsedQuicVersion version); + // Buffers packet until it can be delivered to a connection. + void BufferEarlyPacket(const ReceivedPacketInfo& packet_info); - // Called when |current_packet_| is a CHLO packet. Creates a new connection - // and delivers any buffered packets for that connection id. - void ProcessChlo(PacketHeaderFormat form, ParsedQuicVersion version); - - // Returns the actual client address of the current packet. - // This function should only be called once per packet at the very beginning - // of ProcessPacket(), its result is saved to |current_client_address_| while - // the packet is being processed. - // By default, this function returns |current_peer_address_|, subclasses have - // the option to override this function to return a different address. - virtual const QuicSocketAddress GetClientAddress() const; + // Called when |packet_info| is a CHLO packet. Creates a new connection and + // delivers any buffered packets for that connection id. + void ProcessChlo(const std::string& alpn, ReceivedPacketInfo* packet_info); // Return true if dispatcher wants to destroy session outside of // OnConnectionClosed() call stack. @@ -208,20 +190,6 @@ const ParsedQuicVersionVector& GetSupportedVersions(); - QuicConnectionId current_server_connection_id() const { - return current_server_connection_id_; - } - const QuicSocketAddress& current_self_address() const { - return current_self_address_; - } - const QuicSocketAddress& current_peer_address() const { - return current_peer_address_; - } - const QuicSocketAddress& current_client_address() const { - return current_client_address_; - } - const QuicReceivedPacket& current_packet() const { return *current_packet_; } - const QuicConfig& config() const { return *config_; } const QuicCryptoServerConfig* crypto_config() const { return crypto_config_; } @@ -250,16 +218,14 @@ // Called by MaybeDispatchPacket when current packet cannot be dispatched. // Used by subclasses to conduct specific logic to dispatch packet. Returns // true if packet is successfully dispatched. - virtual bool OnFailedToDispatchPacket( - QuicConnectionId destination_connection_id); + virtual bool OnFailedToDispatchPacket(const ReceivedPacketInfo& packet_info); // Called when a new connection starts to be handled by this dispatcher. // Either this connection is created or its packets is buffered while waiting // for CHLO. Returns true if a new connection should be created or its packets // should be buffered, false otherwise. virtual bool ShouldCreateOrBufferPacketForConnection( - QuicConnectionId server_connection_id, - bool ietf_quic); + const ReceivedPacketInfo& packet_info); bool HasBufferedPackets(QuicConnectionId server_connection_id); @@ -316,11 +282,9 @@ typedef QuicUnorderedSet<QuicConnectionId, QuicConnectionIdHash> QuicConnectionIdSet; - // Calls ValidityChecks and then ProcessUnauthenticatedHeaderFate. - void ProcessHeader(PacketHeaderFormat form, - bool version_flag, - ParsedQuicVersion version, - QuicConnectionId destination_connection_id); + // TODO(fayang): Consider to rename this function to + // ProcessValidatedPacketWithUnknownConnectionId. + void ProcessHeader(ReceivedPacketInfo* packet_info); // Deliver |packets| to |session| for further processing. void DeliverPacketsToSession( @@ -382,20 +346,11 @@ // them. QuicBufferedPacketStore buffered_packets_; - // Information about the packet currently being handled. - QuicSocketAddress current_client_address_; - QuicSocketAddress current_peer_address_; - QuicSocketAddress current_self_address_; - const QuicReceivedPacket* current_packet_; - // If |current_packet_| is a CHLO packet, the extracted alpn. - std::string current_alpn_; - QuicConnectionId current_server_connection_id_; - // Used to get the supported versions based on flag. Does not own. QuicVersionManager* version_manager_; - // The last error set by SetLastError(), which is called by - // framer_visitor_->OnError(). + // The last error set by SetLastError(). + // TODO(fayang): consider removing last_error_. QuicErrorCode last_error_; // A backward counter of how many new sessions can be create within current @@ -415,13 +370,11 @@ // This is also used to signal an error when a long header packet with // different destination connection ID length is received when // should_update_expected_server_connection_id_length_ is false and packet's - // version does not allow variable length connection ID. Used when no_framer_ - // is true. + // version does not allow variable length connection ID. uint8_t expected_server_connection_id_length_; // If true, change expected_server_connection_id_length_ to be the received - // destination connection ID length of all IETF long headers. Used when - // no_framer_ is true. + // destination connection ID length of all IETF long headers. bool should_update_expected_server_connection_id_length_; };
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc index b38654f..8e014b0 100644 --- a/quic/core/quic_dispatcher_test.cc +++ b/quic/core/quic_dispatcher_test.cc
@@ -128,8 +128,8 @@ QuicStringPiece alpn, const quic::ParsedQuicVersion& version)); - MOCK_METHOD2(ShouldCreateOrBufferPacketForConnection, - bool(QuicConnectionId connection_id, bool ietf_quic)); + MOCK_METHOD1(ShouldCreateOrBufferPacketForConnection, + bool(const ReceivedPacketInfo& packet_info)); struct TestQuicPerPacketContext : public QuicPerPacketContext { std::string custom_packet_context; @@ -150,9 +150,6 @@ std::string custom_packet_context_; - using QuicDispatcher::current_client_address; - using QuicDispatcher::current_peer_address; - using QuicDispatcher::current_self_address; using QuicDispatcher::SetAllowShortInitialServerConnectionIds; using QuicDispatcher::writer; }; @@ -214,7 +211,7 @@ // Set the counter to some value to start with. QuicDispatcherPeer::set_new_sessions_allowed_per_event_loop( dispatcher_.get(), kMaxNumSessionsToCreate); - ON_CALL(*dispatcher_, ShouldCreateOrBufferPacketForConnection(_, _)) + ON_CALL(*dispatcher_, ShouldCreateOrBufferPacketForConnection(_)) .WillByDefault(Return(true)); } @@ -382,7 +379,8 @@ ValidatePacket(connection_id, packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(connection_id, _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(connection_id))); ProcessPacket(client_address, connection_id, true, version, SerializeCHLO(), CONNECTION_ID_PRESENT, PACKET_4BYTE_PACKET_NUMBER, 1); } @@ -434,14 +432,13 @@ ValidatePacket(TestConnectionId(1), packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(TestConnectionId(1), _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(1)))); ProcessPacket( client_address, TestConnectionId(1), true, ParsedQuicVersion(PROTOCOL_TLS1_3, CurrentSupportedVersions().front().transport_version), SerializeCHLO(), CONNECTION_ID_PRESENT, PACKET_4BYTE_PACKET_NUMBER, 1); - EXPECT_EQ(client_address, dispatcher_->current_peer_address()); - EXPECT_EQ(server_address_, dispatcher_->current_self_address()); } TEST_F(QuicDispatcherTest, ProcessPackets) { @@ -460,10 +457,9 @@ ValidatePacket(TestConnectionId(1), packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(TestConnectionId(1), _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(1)))); ProcessPacket(client_address, TestConnectionId(1), true, SerializeCHLO()); - EXPECT_EQ(client_address, dispatcher_->current_peer_address()); - EXPECT_EQ(server_address_, dispatcher_->current_self_address()); EXPECT_CALL(*dispatcher_, CreateQuicSession(TestConnectionId(2), client_address, @@ -478,7 +474,8 @@ ValidatePacket(TestConnectionId(2), packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(TestConnectionId(2), _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(2)))); ProcessPacket(client_address, TestConnectionId(2), true, SerializeCHLO()); EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()), @@ -510,7 +507,8 @@ ValidatePacket(TestConnectionId(1), packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(TestConnectionId(1), _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(1)))); ProcessPacket( client_address, TestConnectionId(1), true, ParsedQuicVersion(PROTOCOL_QUIC_CRYPTO, @@ -523,8 +521,6 @@ ParsedQuicVersion(PROTOCOL_QUIC_CRYPTO, CurrentSupportedVersions().front().transport_version), "", CONNECTION_ID_PRESENT, PACKET_1BYTE_PACKET_NUMBER, 256); - EXPECT_EQ(client_address, dispatcher_->current_peer_address()); - EXPECT_EQ(server_address_, dispatcher_->current_self_address()); } TEST_F(QuicDispatcherTest, StatelessVersionNegotiation) { @@ -623,7 +619,8 @@ }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(TestConnectionId(1), _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(1)))); ProcessPacket(client_address, TestConnectionId(1), true, SerializeCHLO()); EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()), @@ -651,7 +648,8 @@ }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(TestConnectionId(1), _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(1)))); ProcessPacket(client_address, connection_id, true, SerializeCHLO()); // Now close the connection, which should add it to the time wait list. @@ -718,10 +716,9 @@ ValidatePacket(bad_connection_id, packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(bad_connection_id, _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(bad_connection_id))); ProcessPacket(client_address, bad_connection_id, true, SerializeCHLO()); - EXPECT_EQ(client_address, dispatcher_->current_peer_address()); - EXPECT_EQ(server_address_, dispatcher_->current_self_address()); } // Makes sure zero-byte connection IDs are replaced by 8-byte ones. @@ -757,10 +754,9 @@ ValidatePacket(bad_connection_id, packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(bad_connection_id, _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(bad_connection_id))); ProcessPacket(client_address, bad_connection_id, true, SerializeCHLO()); - EXPECT_EQ(client_address, dispatcher_->current_peer_address()); - EXPECT_EQ(server_address_, dispatcher_->current_self_address()); } // Makes sure TestConnectionId(1) creates a new connection and @@ -789,10 +785,9 @@ ValidatePacket(TestConnectionId(1), packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(TestConnectionId(1), _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(1)))); ProcessPacket(client_address, TestConnectionId(1), true, SerializeCHLO()); - EXPECT_EQ(client_address, dispatcher_->current_peer_address()); - EXPECT_EQ(server_address_, dispatcher_->current_self_address()); EXPECT_CALL(*dispatcher_, CreateQuicSession(fixed_connection_id, client_address, @@ -808,7 +803,8 @@ ValidatePacket(bad_connection_id, packet); }))); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(bad_connection_id, _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(bad_connection_id))); ProcessPacket(client_address, bad_connection_id, true, SerializeCHLO()); EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()), @@ -876,12 +872,11 @@ // A packet whose packet number is the largest that is allowed to start a // connection. EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(connection_id, _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(connection_id))); ProcessPacket(client_address, connection_id, true, SerializeCHLO(), CONNECTION_ID_PRESENT, PACKET_4BYTE_PACKET_NUMBER, QuicDispatcher::kMaxReasonableInitialPacketNumber); - EXPECT_EQ(client_address, dispatcher_->current_peer_address()); - EXPECT_EQ(server_address_, dispatcher_->current_self_address()); } TEST_F(QuicDispatcherTest, SupportedTransportVersionsChangeInFlight) { @@ -1006,8 +1001,9 @@ .WillOnce(WithArg<2>(Invoke([this](const QuicEncryptedPacket& packet) { ValidatePacket(TestConnectionId(1), packet); }))); - EXPECT_CALL(*dispatcher_, ShouldCreateOrBufferPacketForConnection( - TestConnectionId(1), _)); + EXPECT_CALL(*dispatcher_, + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(1)))); ProcessPacket(client_address, TestConnectionId(1), true, SerializeCHLO()); EXPECT_CALL(*dispatcher_, @@ -1021,8 +1017,9 @@ .WillOnce(WithArg<2>(Invoke([this](const QuicEncryptedPacket& packet) { ValidatePacket(TestConnectionId(2), packet); }))); - EXPECT_CALL(*dispatcher_, ShouldCreateOrBufferPacketForConnection( - TestConnectionId(2), _)); + EXPECT_CALL(*dispatcher_, + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(2)))); ProcessPacket(client_address, TestConnectionId(2), true, SerializeCHLO()); blocked_list_ = QuicDispatcherPeer::GetWriteBlockedList(dispatcher_.get()); @@ -1301,8 +1298,8 @@ QuicConnectionId conn_id = TestConnectionId(1); // A bunch of non-CHLO should be buffered upon arrival, and the first one // should trigger ShouldCreateOrBufferPacketForConnection(). - EXPECT_CALL(*dispatcher_, ShouldCreateOrBufferPacketForConnection(conn_id, _)) - .Times(1); + EXPECT_CALL(*dispatcher_, ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(conn_id))); for (size_t i = 1; i <= kDefaultMaxUndecryptablePackets + 1; ++i) { ProcessPacket(client_address, conn_id, true, QuicStrCat("data packet ", i + 1), CONNECTION_ID_PRESENT, @@ -1343,7 +1340,8 @@ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), i); QuicConnectionId conn_id = TestConnectionId(i); EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(conn_id, _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(conn_id))); ProcessPacket(client_address, conn_id, true, QuicStrCat("data packet on connection ", i), CONNECTION_ID_PRESENT, PACKET_4BYTE_PACKET_NUMBER, @@ -1364,7 +1362,8 @@ QuicConnectionId conn_id = TestConnectionId(i); if (i == kNumConnections) { EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(conn_id, _)); + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(conn_id))); } EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, client_address, QuicStringPiece(), _)) @@ -1391,8 +1390,8 @@ TEST_F(BufferedPacketStoreTest, DeliverEmptyPackets) { QuicConnectionId conn_id = TestConnectionId(1); QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); - EXPECT_CALL(*dispatcher_, - ShouldCreateOrBufferPacketForConnection(conn_id, _)); + EXPECT_CALL(*dispatcher_, ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(conn_id))); EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, client_address, QuicStringPiece(), _)) .WillOnce(testing::Return(CreateSession( @@ -1473,8 +1472,10 @@ const size_t kNumCHLOs = kMaxNumSessionsToCreate + kDefaultMaxConnectionsInStore + 1; for (uint64_t conn_id = 1; conn_id <= kNumCHLOs; ++conn_id) { - EXPECT_CALL(*dispatcher_, ShouldCreateOrBufferPacketForConnection( - TestConnectionId(conn_id), _)); + EXPECT_CALL( + *dispatcher_, + ShouldCreateOrBufferPacketForConnection( + ReceivedPacketInfoConnectionIdEquals(TestConnectionId(conn_id)))); if (conn_id <= kMaxNumSessionsToCreate) { EXPECT_CALL(*dispatcher_, CreateQuicSession(TestConnectionId(conn_id), client_addr_,
diff --git a/quic/core/quic_packets.cc b/quic/core/quic_packets.cc index 0b1c05d..7abf303 100644 --- a/quic/core/quic_packets.cc +++ b/quic/core/quic_packets.cc
@@ -13,6 +13,7 @@ #include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" #include "net/third_party/quiche/src/quic/platform/api/quic_str_cat.h" #include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_string_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_text_utils.h" namespace quic { @@ -487,4 +488,41 @@ return dst_buffer; } +ReceivedPacketInfo::ReceivedPacketInfo(const QuicSocketAddress& self_address, + const QuicSocketAddress& peer_address, + const QuicReceivedPacket& packet) + : self_address(self_address), + peer_address(peer_address), + packet(packet), + form(GOOGLE_QUIC_PACKET), + version_flag(false), + version_label(0), + version(PROTOCOL_UNSUPPORTED, QUIC_VERSION_UNSUPPORTED), + destination_connection_id(EmptyQuicConnectionId()), + source_connection_id(EmptyQuicConnectionId()) {} + +ReceivedPacketInfo::~ReceivedPacketInfo() {} + +std::string ReceivedPacketInfo::ToString() const { + std::string output = + QuicStrCat("{ self_address: ", self_address.ToString(), + ", peer_address: ", peer_address.ToString(), + ", packet_length: ", packet.length(), + ", header_format: ", form, ", version_flag: ", version_flag); + if (version_flag) { + QuicStrAppend(&output, ", version: ", ParsedQuicVersionToString(version)); + } + QuicStrAppend( + &output, + ", destination_connection_id: ", destination_connection_id.ToString(), + ", source_connection_id: ", source_connection_id.ToString(), " }\n"); + return output; +} + +std::ostream& operator<<(std::ostream& os, + const ReceivedPacketInfo& packet_info) { + os << packet_info.ToString(); + return os; +} + } // namespace quic
diff --git a/quic/core/quic_packets.h b/quic/core/quic_packets.h index b1964e7..cf6099e 100644 --- a/quic/core/quic_packets.h +++ b/quic/core/quic_packets.h
@@ -402,6 +402,35 @@ virtual ~QuicPerPacketContext() {} }; +// ReceivedPacketInfo comprises information obtained by parsing the unencrypted +// bytes of a received packet. +struct QUIC_EXPORT_PRIVATE ReceivedPacketInfo { + ReceivedPacketInfo(const QuicSocketAddress& self_address, + const QuicSocketAddress& peer_address, + const QuicReceivedPacket& packet); + ReceivedPacketInfo(const ReceivedPacketInfo& other) = default; + + ~ReceivedPacketInfo(); + + std::string ToString() const; + + QUIC_EXPORT_PRIVATE friend std::ostream& operator<<( + std::ostream& os, + const ReceivedPacketInfo& packet_info); + + const QuicSocketAddress& self_address; + const QuicSocketAddress& peer_address; + const QuicReceivedPacket& packet; + + // Fields below are populated by QuicFramer::ProcessPacketDispatcher. + PacketHeaderFormat form; + bool version_flag; + QuicVersionLabel version_label; + ParsedQuicVersion version; + QuicConnectionId destination_connection_id; + QuicConnectionId source_connection_id; +}; + } // namespace quic #endif // QUICHE_QUIC_CORE_QUIC_PACKETS_H_
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index db103de..4acc275 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -1180,6 +1180,15 @@ QuicStringPiece message_data, QuicMemSliceStorage* storage); +// Used to compare ReceivedPacketInfo. +MATCHER_P(ReceivedPacketInfoEquals, info, "") { + return info.ToString() == arg.ToString(); +} + +MATCHER_P(ReceivedPacketInfoConnectionIdEquals, destination_connection_id, "") { + return arg.destination_connection_id == destination_connection_id; +} + } // namespace test } // namespace quic