Remove framer object from quic_dispatcher and make dispatcher no longer inherit from QuicFramerVisitorInterface. gfe-relnote: Deprecate gfe2_restart_flag_quic_no_framer_object_in_dispatcher. PiperOrigin-RevId: 253997085 Change-Id: I46b884eddfee471f22ec32b35288a2be69521728
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index 7ad7bca..37f1901 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -200,23 +200,13 @@ buffered_packets_(this, helper_->GetClock(), alarm_factory_.get()), current_packet_(nullptr), version_manager_(version_manager), - framer_(GetSupportedVersions(), - /*unused*/ QuicTime::Zero(), - Perspective::IS_SERVER, - expected_server_connection_id_length), last_error_(QUIC_NO_ERROR), new_sessions_allowed_per_event_loop_(0u), accept_new_connections_(true), allow_short_initial_server_connection_ids_(false), - last_version_label_(0), expected_server_connection_id_length_( expected_server_connection_id_length), - should_update_expected_server_connection_id_length_(false), - no_framer_(GetQuicRestartFlag(quic_no_framer_object_in_dispatcher)) { - if (!no_framer_) { - framer_.set_visitor(this); - } -} + should_update_expected_server_connection_id_length_(false) {} QuicDispatcher::~QuicDispatcher() { session_map_.clear(); @@ -241,22 +231,16 @@ // GetClientAddress must be called after current_peer_address_ is set. current_client_address_ = GetClientAddress(); current_packet_ = &packet; - if (!no_framer_) { - // ProcessPacket will cause the packet to be dispatched in - // OnUnauthenticatedPublicHeader, or sent to the time wait list manager - // in OnUnauthenticatedHeader. - framer_.ProcessPacket(packet); - // TODO(rjshade): Return a status describing if/why a packet was dropped, - // and log somehow. Maybe expose as a varz. - return; - } - QUIC_RESTART_FLAG_COUNT(quic_no_framer_object_in_dispatcher); - QuicPacketHeader header; + + PacketHeaderFormat form = GOOGLE_QUIC_PACKET; + bool version_flag = false; + QuicVersionLabel version_label = 0; + QuicConnectionId destination_connection_id = EmptyQuicConnectionId(); + QuicConnectionId source_connection_id = EmptyQuicConnectionId(); std::string detailed_error; const QuicErrorCode error = QuicFramer::ProcessPacketDispatcher( - packet, expected_server_connection_id_length_, &header.form, - &header.version_flag, &last_version_label_, - &header.destination_connection_id, &header.source_connection_id, + packet, expected_server_connection_id_length_, &form, &version_flag, + &version_label, &destination_connection_id, &source_connection_id, &detailed_error); if (error != QUIC_NO_ERROR) { // Packet has framing error. @@ -264,28 +248,26 @@ QUIC_DLOG(ERROR) << detailed_error; return; } - header.version = ParseQuicVersionLabel(last_version_label_); - if (header.destination_connection_id.length() != + ParsedQuicVersion version = ParseQuicVersionLabel(version_label); + if (destination_connection_id.length() != expected_server_connection_id_length_ && !should_update_expected_server_connection_id_length_ && !QuicUtils::VariableLengthConnectionIdAllowedForVersion( - header.version.transport_version)) { + 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_ = - header.destination_connection_id.length(); + expected_server_connection_id_length_ = destination_connection_id.length(); } - // TODO(fayang): Instead of passing in QuicPacketHeader, pass format, - // version_flag, version and destination_connection_id. Combine - // OnUnauthenticatedPublicHeader and OnUnauthenticatedHeader to a single - // function when deprecating quic_no_framer_object_in_dispatcher. - if (!OnUnauthenticatedPublicHeader(header)) { + + if (MaybeDispatchPacket(form, version_flag, version_label, version, + destination_connection_id, source_connection_id)) { + // Packet has been dropped or successfully dispatched, stop processing. return; } - OnUnauthenticatedHeader(header); + 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. } @@ -293,10 +275,7 @@ QuicConnectionId QuicDispatcher::MaybeReplaceServerConnectionId( QuicConnectionId server_connection_id, ParsedQuicVersion version) { - const uint8_t expected_server_connection_id_length = - no_framer_ ? expected_server_connection_id_length_ - : framer_.GetExpectedServerConnectionIdLength(); - if (server_connection_id.length() == expected_server_connection_id_length) { + if (server_connection_id.length() == expected_server_connection_id_length_) { return server_connection_id; } DCHECK(QuicUtils::VariableLengthConnectionIdAllowedForVersion( @@ -308,7 +287,7 @@ QuicConnectionId new_connection_id = session_helper_->GenerateConnectionIdForReject(version.transport_version, server_connection_id); - DCHECK_EQ(expected_server_connection_id_length, new_connection_id.length()); + DCHECK_EQ(expected_server_connection_id_length_, new_connection_id.length()); connection_id_map_.insert( std::make_pair(server_connection_id, new_connection_id)); QUIC_DLOG(INFO) << "Replacing incoming connection ID " << server_connection_id @@ -316,45 +295,41 @@ return new_connection_id; } -bool QuicDispatcher::OnUnauthenticatedPublicHeader( - const QuicPacketHeader& header) { - current_server_connection_id_ = header.destination_connection_id; +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; // 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) { - return false; + return true; } - // The dispatcher requires the connection ID to be present in order to - // look up the matching QuicConnection, so we error out if it is absent. - if (header.destination_connection_id_included != CONNECTION_ID_PRESENT) { - return false; - } - QuicConnectionId server_connection_id = header.destination_connection_id; + QuicConnectionId server_connection_id = 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 // connection ID, the dispatcher picks a new one of its expected length. // Therefore we should never receive a connection ID that is smaller // than 64 bits and smaller than what we expect. - const uint8_t expected_server_connection_id_length = - no_framer_ ? expected_server_connection_id_length_ - : framer_.GetExpectedServerConnectionIdLength(); if (server_connection_id.length() < kQuicMinimumInitialConnectionIdLength && - server_connection_id.length() < expected_server_connection_id_length && + server_connection_id.length() < expected_server_connection_id_length_ && !allow_short_initial_server_connection_ids_) { - DCHECK(header.version_flag); + DCHECK(version_flag); DCHECK(QuicUtils::VariableLengthConnectionIdAllowedForVersion( - header.version.transport_version)); + version.transport_version)); QUIC_DLOG(INFO) << "Packet with short destination connection ID " << server_connection_id << " expected " - << static_cast<int>(expected_server_connection_id_length); - ProcessUnauthenticatedHeaderFate(kFateTimeWait, server_connection_id, - header.form, header.version_flag, - header.version); - return false; + << static_cast<int>(expected_server_connection_id_length_); + ProcessUnauthenticatedHeaderFate(kFateTimeWait, server_connection_id, form, + version_flag, version); + return true; } // Packets with connection IDs for active connections are processed @@ -364,13 +339,13 @@ DCHECK(!buffered_packets_.HasBufferedPackets(server_connection_id)); it->second->ProcessUdpPacket(current_self_address_, current_peer_address_, *current_packet_); - return false; + return true; } if (buffered_packets_.HasChloForConnection(server_connection_id)) { - BufferEarlyPacket(server_connection_id, header.form != GOOGLE_QUIC_PACKET, - header.version); - return false; + BufferEarlyPacket(server_connection_id, form != GOOGLE_QUIC_PACKET, + version); + return true; } // Check if we are buffering packets for this connection ID @@ -378,89 +353,64 @@ temporarily_buffered_connections_.end()) { // This packet was received while the a CHLO for the same connection ID was // being processed. Buffer it. - BufferEarlyPacket(server_connection_id, header.form != GOOGLE_QUIC_PACKET, - header.version); - return false; + BufferEarlyPacket(server_connection_id, form != GOOGLE_QUIC_PACKET, + version); + return true; } - if (!OnUnauthenticatedUnknownPublicHeader(header)) { - return false; - } - - // If the packet is a public reset for a connection ID that is not active, - // there is nothing we must do or can do. - if (header.reset_flag) { - return false; + if (OnFailedToDispatchPacket(destination_connection_id)) { + 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_, - header.destination_connection_id, header.form, GetPerPacketContext()); - return false; + current_self_address_, current_peer_address_, destination_connection_id, + form, GetPerPacketContext()); + return true; } // The packet has an unknown connection ID. // Unless the packet provides a version, assume that we can continue // processing using our preferred version. - ParsedQuicVersion version = GetSupportedVersions().front(); - if (header.version_flag) { - ParsedQuicVersion packet_version = header.version; - if (!no_framer_ && framer_.supported_versions() != GetSupportedVersions()) { - // Reset framer's version if version flags change in flight. - framer_.SetSupportedVersions(GetSupportedVersions()); - } - if (!IsSupportedVersion(packet_version)) { - if (ShouldCreateSessionForUnknownVersion( - no_framer_ ? last_version_label_ - : framer_.last_version_label())) { - return true; + if (version_flag) { + if (!IsSupportedVersion(version)) { + if (ShouldCreateSessionForUnknownVersion(version_label)) { + return false; } if (!crypto_config()->validate_chlo_size() || current_packet_->length() >= kMinPacketSizeForVersionNegotiation) { // Since the version is not supported, send a version negotiation // packet and stop processing the current packet. - QuicConnectionId client_connection_id = header.source_connection_id; + QuicConnectionId client_connection_id = source_connection_id; time_wait_list_manager()->SendVersionNegotiationPacket( server_connection_id, client_connection_id, - header.form != GOOGLE_QUIC_PACKET, GetSupportedVersions(), + form != GOOGLE_QUIC_PACKET, GetSupportedVersions(), current_self_address_, current_peer_address_, GetPerPacketContext()); } - return false; + return true; } - version = packet_version; - } - if (!no_framer_) { - // Set the framer's version and continue processing. - framer_.set_version(version); } - if (version.HasHeaderProtection()) { - ProcessHeader(header); - return false; - } - return true; -} - -bool QuicDispatcher::OnUnauthenticatedHeader(const QuicPacketHeader& header) { - ProcessHeader(header); return false; } -void QuicDispatcher::ProcessHeader(const QuicPacketHeader& header) { - QuicConnectionId server_connection_id = header.destination_connection_id; +void QuicDispatcher::ProcessHeader(PacketHeaderFormat form, + bool version_flag, + ParsedQuicVersion version, + QuicConnectionId destination_connection_id) { + QuicConnectionId server_connection_id = 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(header); + QuicPacketFate fate = + ValidityChecks(version_flag, version, destination_connection_id); if (fate == kFateProcess) { - if (header.version.handshake_protocol == PROTOCOL_TLS1_3) { - ProcessUnauthenticatedHeaderFate(kFateProcess, server_connection_id, - header.form, header.version_flag, - header.version); + if (version.handshake_protocol == PROTOCOL_TLS1_3) { + ProcessUnauthenticatedHeaderFate(kFateProcess, server_connection_id, form, + version_flag, version); return; // TODO(nharper): Support buffering non-ClientHello packets when using // TLS. @@ -473,21 +423,19 @@ &alpn_extractor, server_connection_id.length())) { // Buffer non-CHLO packets. - ProcessUnauthenticatedHeaderFate(kFateBuffer, server_connection_id, - header.form, header.version_flag, - header.version); + ProcessUnauthenticatedHeaderFate(kFateBuffer, server_connection_id, form, + version_flag, version); return; } current_alpn_ = alpn_extractor.ConsumeAlpn(); - ProcessUnauthenticatedHeaderFate(kFateProcess, server_connection_id, - header.form, header.version_flag, - header.version); + ProcessUnauthenticatedHeaderFate(kFateProcess, server_connection_id, form, + version_flag, version); return; } // Fate is already known. - ProcessUnauthenticatedHeaderFate(fate, server_connection_id, header.form, - header.version_flag, header.version); + ProcessUnauthenticatedHeaderFate(fate, server_connection_id, form, + version_flag, version); } void QuicDispatcher::ProcessUnauthenticatedHeaderFate( @@ -534,7 +482,9 @@ } QuicDispatcher::QuicPacketFate QuicDispatcher::ValidityChecks( - const QuicPacketHeader& header) { + bool version_flag, + ParsedQuicVersion /*version*/, + QuicConnectionId destination_connection_id) { // 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 @@ -548,29 +498,14 @@ // 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 (!header.version_flag) { + if (!version_flag) { QUIC_DLOG(INFO) << "Packet without version arrived for unknown connection ID " - << header.destination_connection_id; + << destination_connection_id; return kFateTimeWait; } - if (no_framer_) { - // Let the connection parse and validate packet number. - return kFateProcess; - } - - // initial packet number of 0 is always invalid. - if (!framer_.version().HasHeaderProtection()) { - if (!header.packet_number.IsInitialized()) { - return kFateTimeWait; - } - // Accepting Initial Packet Numbers in 1...((2^31)-1) range... check - // maximum accordingly. - if (header.packet_number > MaxRandomInitialPacketNumber()) { - return kFateTimeWait; - } - } + // Let the connection parse and validate packet number. return kFateProcess; } @@ -802,201 +737,11 @@ &termination_packets); } -void QuicDispatcher::OnPacket() {} - -void QuicDispatcher::OnError(QuicFramer* framer) { - QuicErrorCode error = framer->error(); - SetLastError(error); - QUIC_DLOG(INFO) << QuicErrorCodeToString(error); -} - bool QuicDispatcher::ShouldCreateSessionForUnknownVersion( QuicVersionLabel /*version_label*/) { return false; } -bool QuicDispatcher::OnProtocolVersionMismatch( - ParsedQuicVersion /*received_version*/, - PacketHeaderFormat /*form*/) { - DCHECK(!no_framer_); - QUIC_BUG_IF( - !time_wait_list_manager_->IsConnectionIdInTimeWait( - current_server_connection_id_) && - !ShouldCreateSessionForUnknownVersion(framer_.last_version_label())) - << "Unexpected version mismatch: " - << QuicVersionLabelToString(framer_.last_version_label()); - - // Keep processing after protocol mismatch - this will be dealt with by the - // time wait list or connection that we will create. - return true; -} - -void QuicDispatcher::OnPublicResetPacket( - const QuicPublicResetPacket& /*packet*/) { - DCHECK(false); -} - -void QuicDispatcher::OnVersionNegotiationPacket( - const QuicVersionNegotiationPacket& /*packet*/) { - DCHECK(false); -} - -void QuicDispatcher::OnRetryPacket(QuicConnectionId /*original_connection_id*/, - QuicConnectionId /*new_connection_id*/, - QuicStringPiece /*retry_token*/) { - DCHECK(false); -} - -void QuicDispatcher::OnDecryptedPacket(EncryptionLevel /*level*/) { - DCHECK(false); -} - -bool QuicDispatcher::OnPacketHeader(const QuicPacketHeader& /*header*/) { - DCHECK(false); - return false; -} - -void QuicDispatcher::OnCoalescedPacket(const QuicEncryptedPacket& /*packet*/) { - DCHECK(false); -} - -bool QuicDispatcher::OnStreamFrame(const QuicStreamFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnCryptoFrame(const QuicCryptoFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnAckFrameStart(QuicPacketNumber /*largest_acked*/, - QuicTime::Delta /*ack_delay_time*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnAckRange(QuicPacketNumber /*start*/, - QuicPacketNumber /*end*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnAckTimestamp(QuicPacketNumber /*packet_number*/, - QuicTime /*timestamp*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnAckFrameEnd(QuicPacketNumber /*start*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnStopWaitingFrame(const QuicStopWaitingFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnPaddingFrame(const QuicPaddingFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnPingFrame(const QuicPingFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnRstStreamFrame(const QuicRstStreamFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnConnectionCloseFrame( - const QuicConnectionCloseFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnMaxStreamsFrame(const QuicMaxStreamsFrame& /*frame*/) { - return true; -} - -bool QuicDispatcher::OnStreamsBlockedFrame( - const QuicStreamsBlockedFrame& /*frame*/) { - return true; -} - -bool QuicDispatcher::OnStopSendingFrame(const QuicStopSendingFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnPathChallengeFrame( - const QuicPathChallengeFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnPathResponseFrame( - const QuicPathResponseFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnGoAwayFrame(const QuicGoAwayFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnWindowUpdateFrame( - const QuicWindowUpdateFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnBlockedFrame(const QuicBlockedFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnNewConnectionIdFrame( - const QuicNewConnectionIdFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnRetireConnectionIdFrame( - const QuicRetireConnectionIdFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnNewTokenFrame(const QuicNewTokenFrame& /*frame*/) { - DCHECK(false); - return false; -} - -bool QuicDispatcher::OnMessageFrame(const QuicMessageFrame& /*frame*/) { - DCHECK(false); - return false; -} - -void QuicDispatcher::OnPacketComplete() { - DCHECK(false); -} - -bool QuicDispatcher::IsValidStatelessResetToken(QuicUint128 /*token*/) const { - DCHECK(false); - return false; -} - -void QuicDispatcher::OnAuthenticatedIetfStatelessResetPacket( - const QuicIetfStatelessResetPacket& /*packet*/) { - DCHECK(false); -} - void QuicDispatcher::OnExpiredPackets( QuicConnectionId server_connection_id, BufferedPacketList early_arrived_packets) { @@ -1161,9 +906,9 @@ last_error_ = error; } -bool QuicDispatcher::OnUnauthenticatedUnknownPublicHeader( - const QuicPacketHeader& /*header*/) { - return true; +bool QuicDispatcher::OnFailedToDispatchPacket( + QuicConnectionId /*destination_connection_id*/) { + return false; } const QuicTransportVersionVector& @@ -1184,16 +929,7 @@ } } -void QuicDispatcher::DisableFlagValidation() { - if (!no_framer_) { - framer_.set_validate_flags(false); - } -} - bool QuicDispatcher::IsSupportedVersion(const ParsedQuicVersion version) { - if (!no_framer_) { - return framer_.IsSupportedVersion(version); - } for (const ParsedQuicVersion& supported_version : version_manager_->GetSupportedVersions()) { if (version == supported_version) {
diff --git a/quic/core/quic_dispatcher.h b/quic/core/quic_dispatcher.h index 19ecd63..d3d73b0 100644 --- a/quic/core/quic_dispatcher.h +++ b/quic/core/quic_dispatcher.h
@@ -36,7 +36,6 @@ class QuicDispatcher : public QuicTimeWaitListManager::Visitor, public ProcessPacketInterface, - public QuicFramerVisitorInterface, public QuicBufferedPacketStore::VisitorInterface { public: // Ideally we'd have a linked_hash_set: the boolean is unused. @@ -129,66 +128,6 @@ "kMaxReasonableInitialPacketNumber is unreasonably small " "relative to kInitialCongestionWindow."); - // QuicFramerVisitorInterface implementation. Not expected to be called - // outside of this class. - // TODO(fayang): Make QuicDispatcher no longer implement - // QuicFramerVisitorInterface when deprecating - // quic_no_framer_object_in_dispatcher. - void OnPacket() override; - // Called when the public header has been parsed. Returns false when just the - // public header is enough to dispatch the packet; true if the framer needs to - // continue parsing the packet. - bool OnUnauthenticatedPublicHeader(const QuicPacketHeader& header) override; - // Called when the private header has been parsed. - bool OnUnauthenticatedHeader(const QuicPacketHeader& header) override; - void OnError(QuicFramer* framer) override; - bool OnProtocolVersionMismatch(ParsedQuicVersion received_version, - PacketHeaderFormat form) override; - - // The following methods should never get called because - // OnUnauthenticatedPublicHeader() or OnUnauthenticatedHeader() (whichever - // was called last), will return false and prevent a subsequent invocation - // of these methods. Thus, the payload of the packet is never processed in - // the dispatcher. - void OnPublicResetPacket(const QuicPublicResetPacket& packet) override; - void OnVersionNegotiationPacket( - const QuicVersionNegotiationPacket& packet) override; - void OnRetryPacket(QuicConnectionId original_connection_id, - QuicConnectionId new_connection_id, - QuicStringPiece retry_token) override; - void OnDecryptedPacket(EncryptionLevel level) override; - bool OnPacketHeader(const QuicPacketHeader& header) override; - void OnCoalescedPacket(const QuicEncryptedPacket& packet) override; - bool OnStreamFrame(const QuicStreamFrame& frame) override; - bool OnCryptoFrame(const QuicCryptoFrame& frame) override; - bool OnAckFrameStart(QuicPacketNumber largest_acked, - QuicTime::Delta ack_delay_time) override; - bool OnAckRange(QuicPacketNumber start, QuicPacketNumber end) override; - bool OnAckTimestamp(QuicPacketNumber packet_number, - QuicTime timestamp) override; - bool OnAckFrameEnd(QuicPacketNumber start) override; - bool OnStopWaitingFrame(const QuicStopWaitingFrame& frame) override; - bool OnPaddingFrame(const QuicPaddingFrame& frame) override; - bool OnPingFrame(const QuicPingFrame& frame) override; - bool OnRstStreamFrame(const QuicRstStreamFrame& frame) override; - bool OnConnectionCloseFrame(const QuicConnectionCloseFrame& frame) override; - bool OnStopSendingFrame(const QuicStopSendingFrame& frame) override; - bool OnPathChallengeFrame(const QuicPathChallengeFrame& frame) override; - bool OnPathResponseFrame(const QuicPathResponseFrame& frame) override; - bool OnGoAwayFrame(const QuicGoAwayFrame& frame) override; - bool OnMaxStreamsFrame(const QuicMaxStreamsFrame& frame) override; - bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame) override; - bool OnWindowUpdateFrame(const QuicWindowUpdateFrame& frame) override; - bool OnBlockedFrame(const QuicBlockedFrame& frame) override; - bool OnNewConnectionIdFrame(const QuicNewConnectionIdFrame& frame) override; - bool OnRetireConnectionIdFrame( - const QuicRetireConnectionIdFrame& frame) override; - bool OnNewTokenFrame(const QuicNewTokenFrame& frame) override; - bool OnMessageFrame(const QuicMessageFrame& frame) override; - void OnPacketComplete() override; - bool IsValidStatelessResetToken(QuicUint128 token) const override; - void OnAuthenticatedIetfStatelessResetPacket( - const QuicIetfStatelessResetPacket& packet) override; // QuicBufferedPacketStore::VisitorInterface implementation. void OnExpiredPackets(QuicConnectionId server_connection_id, @@ -207,6 +146,17 @@ QuicStringPiece alpn, const ParsedQuicVersion& version) = 0; + // Tries to validate and dispatch packet based on available information. + // 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); + // Values to be returned by ValidityChecks() to indicate what should be done // with a packet. Fates with greater values are considered to be higher // priority, in that if one validity check indicates a lower-valued fate and @@ -223,10 +173,13 @@ kFateDrop, }; - // This method is called by OnUnauthenticatedHeader on packets not associated - // with a known connection ID. It applies validity checks and returns a + // 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(const QuicPacketHeader& header); + virtual QuicPacketFate ValidityChecks( + bool version_flag, + ParsedQuicVersion version, + QuicConnectionId destination_connection_id); // Create and return the time wait list manager for this dispatcher, which // will be owned by the dispatcher as time_wait_list_manager_ @@ -301,11 +254,11 @@ void SetLastError(QuicErrorCode error); - // Called when the public header has been parsed and the session has been - // looked up, and the session was not found in the active list of sessions. - // Returns false if processing should stop after this call. - virtual bool OnUnauthenticatedUnknownPublicHeader( - const QuicPacketHeader& header); + // 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); // Called when a new connection starts to be handled by this dispatcher. // Either this connection is created or its packets is buffered while waiting @@ -348,18 +301,10 @@ virtual void RestorePerPacketContext( std::unique_ptr<QuicPerPacketContext> /*context*/) {} - // Skip validating that the public flags are set to legal values. - void DisableFlagValidation(); - // If true, our framer will change its expected connection ID length // to the received destination connection ID length of all IETF long headers. void SetShouldUpdateExpectedServerConnectionIdLength( bool should_update_expected_server_connection_id_length) { - if (!no_framer_) { - framer_.SetShouldUpdateExpectedServerConnectionIdLength( - should_update_expected_server_connection_id_length); - return; - } should_update_expected_server_connection_id_length_ = should_update_expected_server_connection_id_length; } @@ -378,9 +323,11 @@ typedef QuicUnorderedSet<QuicConnectionId, QuicConnectionIdHash> QuicConnectionIdSet; - // Based on an unauthenticated packet header |header|, calls ValidityChecks - // and then ProcessUnauthenticatedHeaderFate. - void ProcessHeader(const QuicPacketHeader& header); + // Calls ValidityChecks and then ProcessUnauthenticatedHeaderFate. + void ProcessHeader(PacketHeaderFormat form, + bool version_flag, + ParsedQuicVersion version, + QuicConnectionId destination_connection_id); // Deliver |packets| to |session| for further processing. void DeliverPacketsToSession( @@ -467,8 +414,6 @@ // Used to get the supported versions based on flag. Does not own. QuicVersionManager* version_manager_; - QuicFramer framer_; - // The last error set by SetLastError(), which is called by // framer_visitor_->OnError(). QuicErrorCode last_error_; @@ -485,12 +430,6 @@ // If true they are allowed. bool allow_short_initial_server_connection_ids_; - // The last QUIC version label received. Used when no_framer_ is true. - // TODO(fayang): remove this member variable, instead, add an argument to - // OnUnauthenticatedPublicHeader when deprecating - // quic_no_framer_object_in_dispatcher. - QuicVersionLabel last_version_label_; - // IETF short headers contain a destination connection ID but do not // encode its length. This variable contains the length we expect to read. // This is also used to signal an error when a long header packet with @@ -504,9 +443,6 @@ // destination connection ID length of all IETF long headers. Used when // no_framer_ is true. bool should_update_expected_server_connection_id_length_; - - // Latched value of quic_no_framer_object_in_dispatcher. - const bool no_framer_; }; } // namespace quic
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc index 80b8aee..20c94f1 100644 --- a/quic/core/quic_framer.cc +++ b/quic/core/quic_framer.cc
@@ -464,7 +464,6 @@ error_(QUIC_NO_ERROR), last_serialized_server_connection_id_(EmptyQuicConnectionId()), last_serialized_client_connection_id_(EmptyQuicConnectionId()), - last_version_label_(0), version_(PROTOCOL_UNSUPPORTED, QUIC_VERSION_UNSUPPORTED), supported_versions_(supported_versions), decrypter_level_(ENCRYPTION_INITIAL), @@ -482,7 +481,6 @@ expected_server_connection_id_length_( expected_server_connection_id_length), expected_client_connection_id_length_(0), - should_update_expected_server_connection_id_length_(false), supports_multiple_packet_number_spaces_(false), last_written_packet_number_length_(0) { DCHECK(!supported_versions.empty()); @@ -2356,7 +2354,6 @@ // If the version from the new packet is the same as the version of this // framer, then the public flags should be set to something we understand. // If not, this raises an error. - last_version_label_ = version_label; ParsedQuicVersion version = ParseQuicVersionLabel(version_label); if (version == version_ && public_flags > PACKET_PUBLIC_FLAGS_MAX) { set_detailed_error("Illegal public flags value."); @@ -2568,10 +2565,6 @@ } } } - if (header->long_packet_type != VERSION_NEGOTIATION) { - // Do not save version of version negotiation packet. - last_version_label_ = version_label; - } QUIC_DVLOG(1) << ENDPOINT << "Received IETF long header: " << QuicUtils::QuicLongHeaderTypetoString( @@ -2687,7 +2680,7 @@ if (header->form == IETF_QUIC_LONG_HEADER_PACKET) { if (!ProcessAndValidateIetfConnectionIdLength( reader, header->version, perspective_, - should_update_expected_server_connection_id_length_, + /*should_update_expected_server_connection_id_length=*/false, &expected_server_connection_id_length_, &destination_connection_id_length, &source_connection_id_length, &detailed_error_)) {
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h index 578cfcf..18420f1 100644 --- a/quic/core/quic_framer.h +++ b/quic/core/quic_framer.h
@@ -553,8 +553,6 @@ Perspective perspective() const { return perspective_; } - QuicVersionLabel last_version_label() const { return last_version_label_; } - void set_data_producer(QuicStreamFrameDataProducer* data_producer) { data_producer_ = data_producer; } @@ -565,14 +563,6 @@ return first_sending_packet_number_; } - // If true, QuicFramer will change its expected connection ID length - // to the received destination connection ID length of all IETF long headers. - void SetShouldUpdateExpectedServerConnectionIdLength( - bool should_update_expected_server_connection_id_length) { - should_update_expected_server_connection_id_length_ = - should_update_expected_server_connection_id_length; - } - // The connection ID length the framer expects on incoming IETF short headers // on the server. uint8_t GetExpectedServerConnectionIdLength() { @@ -968,10 +958,6 @@ QuicConnectionId last_serialized_server_connection_id_; // Last client connection ID seen on the wire. QuicConnectionId last_serialized_client_connection_id_; - // The last QUIC version label received. - // TODO(fayang): Remove this when deprecating - // quic_no_framer_object_in_dispatcher. - QuicVersionLabel last_version_label_; // Version of the protocol being used. ParsedQuicVersion version_; // This vector contains QUIC versions which we currently support. @@ -1028,13 +1014,6 @@ uint8_t expected_server_connection_id_length_; uint8_t expected_client_connection_id_length_; - // When this is true, QuicFramer will change - // expected_server_connection_id_length_ to the received destination - // connection ID length of all IETF long headers. - // TODO(fayang): Remove this when deprecating - // quic_no_framer_object_in_dispatcher. - bool should_update_expected_server_connection_id_length_; - // Indicates whether this framer supports multiple packet number spaces. bool supports_multiple_packet_number_spaces_;
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc index 094ffa4..6dd3603 100644 --- a/quic/core/quic_framer_test.cc +++ b/quic/core/quic_framer_test.cc
@@ -13439,125 +13439,10 @@ CheckFramingBoundaries(fragments, QUIC_INVALID_PACKET_HEADER); } -TEST_P(QuicFramerTest, UpdateExpectedConnectionIdLength) { - if (framer_.transport_version() < QUIC_VERSION_46) { - return; - } - SetDecrypterLevel(ENCRYPTION_ZERO_RTT); - framer_.SetShouldUpdateExpectedServerConnectionIdLength(true); - - // clang-format off - unsigned char long_header_packet[] = { - // public flags (long header with packet type ZERO_RTT_PROTECTED and - // 4-byte packet number) - 0xD3, - // version - QUIC_VERSION_BYTES, - // destination connection ID length - 0x60, - // destination connection ID - 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, 0x42, - // packet number - 0x12, 0x34, 0x56, 0x78, - // padding frame - 0x00, - }; - unsigned char long_header_packet99[] = { - // public flags (long header with packet type ZERO_RTT_PROTECTED and - // 4-byte packet number) - 0xD3, - // version - QUIC_VERSION_BYTES, - // destination connection ID length - 0x60, - // destination connection ID - 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, 0x42, - // long header packet length - 0x05, - // packet number - 0x12, 0x34, 0x56, 0x78, - // padding frame - 0x00, - }; - // clang-format on - - if (!QuicVersionHasLongHeaderLengths(framer_.transport_version())) { - EXPECT_TRUE(framer_.ProcessPacket( - QuicEncryptedPacket(AsChars(long_header_packet), - QUIC_ARRAYSIZE(long_header_packet), false))); - } else { - EXPECT_TRUE(framer_.ProcessPacket( - QuicEncryptedPacket(AsChars(long_header_packet99), - QUIC_ARRAYSIZE(long_header_packet99), false))); - } - - EXPECT_EQ(QUIC_NO_ERROR, framer_.error()); - ASSERT_TRUE(visitor_.header_.get()); - EXPECT_EQ(visitor_.header_.get()->destination_connection_id, - FramerTestConnectionIdNineBytes()); - EXPECT_EQ(visitor_.header_.get()->packet_number, - QuicPacketNumber(UINT64_C(0x12345678))); - - SetDecrypterLevel(ENCRYPTION_FORWARD_SECURE); - // clang-format off - unsigned char short_header_packet[] = { - // type (short header, 4 byte packet number) - 0x43, - // connection_id - 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, 0x42, - // packet number - 0x13, 0x37, 0x42, 0x33, - // padding frame - 0x00, - }; - // clang-format on - - QuicEncryptedPacket short_header_encrypted( - AsChars(short_header_packet), QUIC_ARRAYSIZE(short_header_packet), false); - EXPECT_TRUE(framer_.ProcessPacket(short_header_encrypted)); - - EXPECT_EQ(QUIC_NO_ERROR, framer_.error()); - ASSERT_TRUE(visitor_.header_.get()); - EXPECT_EQ(visitor_.header_.get()->destination_connection_id, - FramerTestConnectionIdNineBytes()); - EXPECT_EQ(visitor_.header_.get()->packet_number, - QuicPacketNumber(UINT64_C(0x13374233))); - - PacketHeaderFormat format; - bool version_flag; - QuicConnectionId destination_connection_id, source_connection_id; - QuicVersionLabel version_label; - std::string detailed_error; - EXPECT_EQ(QUIC_NO_ERROR, - QuicFramer::ProcessPacketDispatcher( - QuicEncryptedPacket(AsChars(long_header_packet), - QUIC_ARRAYSIZE(long_header_packet)), - kQuicDefaultConnectionIdLength, &format, &version_flag, - &version_label, &destination_connection_id, - &source_connection_id, &detailed_error)); - EXPECT_EQ(IETF_QUIC_LONG_HEADER_PACKET, format); - EXPECT_TRUE(version_flag); - EXPECT_EQ(9, destination_connection_id.length()); - EXPECT_EQ(FramerTestConnectionIdNineBytes(), destination_connection_id); - EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id); - - EXPECT_EQ( - QUIC_NO_ERROR, - QuicFramer::ProcessPacketDispatcher( - short_header_encrypted, 9, &format, &version_flag, &version_label, - &destination_connection_id, &source_connection_id, &detailed_error)); - EXPECT_EQ(IETF_QUIC_SHORT_HEADER_PACKET, format); - EXPECT_FALSE(version_flag); - EXPECT_EQ(9, destination_connection_id.length()); - EXPECT_EQ(FramerTestConnectionIdNineBytes(), destination_connection_id); - EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id); -} - TEST_P(QuicFramerTest, MultiplePacketNumberSpaces) { if (framer_.transport_version() < QUIC_VERSION_46) { return; } - framer_.SetShouldUpdateExpectedServerConnectionIdLength(true); framer_.EnableMultiplePacketNumberSpacesSupport(); // clang-format off @@ -13568,9 +13453,9 @@ // version QUIC_VERSION_BYTES, // destination connection ID length - 0x60, + 0x50, // destination connection ID - 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, 0x42, + 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, // packet number 0x12, 0x34, 0x56, 0x78, // padding frame @@ -13583,9 +13468,9 @@ // version QUIC_VERSION_BYTES, // destination connection ID length - 0x60, + 0x50, // destination connection ID - 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, 0x42, + 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, // long header packet length 0x05, // packet number @@ -13627,7 +13512,7 @@ // type (short header, 1 byte packet number) 0x40, // connection_id - 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, 0x42, + 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, // packet number 0x79, // padding frame
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc index cfdacca..e2252a6 100644 --- a/quic/core/quic_versions.cc +++ b/quic/core/quic_versions.cc
@@ -440,7 +440,6 @@ SetQuicReloadableFlag(quic_simplify_stop_waiting, true); SetQuicRestartFlag(quic_no_server_conn_ver_negotiation2, true); SetQuicRestartFlag(quic_do_not_override_connection_id, true); - SetQuicRestartFlag(quic_no_framer_object_in_dispatcher, true); SetQuicRestartFlag(quic_use_allocated_connection_ids, true); }