Skip to content

Commit 14e026d

Browse files
incapdnsLucas
andauthored
Refactor stream ID validation and error handling (#71)
* Refactor stream ID validation and error handling Refactor stream ID validation logic and improve error handling for new streams. --------- Co-authored-by: Lucas <lucas@easy-way.app>
1 parent e11c31e commit 14e026d

File tree

1 file changed

+28
-23
lines changed

1 file changed

+28
-23
lines changed

src/connection.rs

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -484,47 +484,49 @@ impl RecvHalfConnection {
484484
let id = frm.stream_id();
485485
let is_server = self.0.flags.get().contains(ConnectionFlags::SERVER);
486486

487+
// 1. Check if ID parity is correct (Client must send odd, Server even)
487488
if is_server && !id.is_client_initiated() {
488489
return Err(Either::Left(ConnectionError::InvalidStreamId(
489490
"Invalid id in received headers frame",
490491
)));
491492
}
492493

493-
// Check id, HTTP2/5.1.1
494+
// 2. CORRECTION: Check logic priority
495+
// First we check if it's an ALREADY EXISTING stream.
496+
// If the stream exists (is OPEN or HALF_CLOSED), we accept the frame regardless of the last_id.
497+
// This allows intermediate trailers and headers.
498+
if let Some(stream) = self.query(id) {
499+
match stream.recv_headers(frm) {
500+
Ok(item) => return Ok(item.map(move |msg| (stream, msg))),
501+
Err(kind) => return Err(Either::Right(StreamErrorInner::new(stream, kind))),
502+
}
503+
}
504+
505+
// 3. Validation of RFC 5.1.1 for NEW streams
506+
// If we've arrived here, the stream does NOT exist on our map.
507+
// Therefore, it must be a newly created stream. The ID must be higher than the last one viewed.
494508
let last_id = self.0.last_id.get();
495-
if last_id > id {
509+
if last_id >= id {
510+
// If the ID is old and the stream was not found above, it's an error (Stream Closed or ID Reuse).
496511
return Err(Either::Left(ConnectionError::InvalidStreamId(
497-
"Invalid id in received headers frame",
512+
"Invalid id in received headers frame (stream closed or ID reused)",
498513
)));
499-
} else if last_id == id {
500-
// If last_id is the same as the current id, it means we have already
501-
// processed the headers associated with this stream id
502-
return if let Some(stream) = self.query(id) {
503-
match stream.recv_headers(frm) {
504-
Ok(item) => Ok(item.map(move |msg| (stream, msg))),
505-
Err(kind) => Err(Either::Right(StreamErrorInner::new(stream, kind))),
506-
}
507-
} else {
508-
Err(Either::Left(ConnectionError::GoAway(
509-
frame::Reason::STREAM_CLOSED,
510-
)))
511-
};
512514
}
515+
516+
// 4. Update last_id (Valid new stream)
513517
self.0.last_id.set(id);
514518

515-
if let Some(stream) = self.query(id) {
516-
match stream.recv_headers(frm) {
517-
Ok(item) => Ok(item.map(move |msg| (stream, msg))),
518-
Err(kind) => Err(Either::Right(StreamErrorInner::new(stream, kind))),
519-
}
520-
} else if !is_server
519+
// 5. Handle specific client closed/pending cases for new streams
520+
if !is_server
521521
&& (!self.0.err_unknown_streams() || self.0.local_pending_reset.is_pending(id))
522522
{
523523
// if client and no stream, then it was closed
524524
self.encode(frame::Reset::new(id, frame::Reason::STREAM_CLOSED));
525525
Ok(None)
526526
} else {
527-
// refuse stream if connection is preparing for disconnect
527+
// 6. New Stream Validation Logic (Disconnect, Max Concurrency, Pseudo Headers)
528+
529+
// Refuse stream if connection is preparing for disconnect
528530
if self
529531
.0
530532
.flags
@@ -536,6 +538,7 @@ impl RecvHalfConnection {
536538
return Ok(None);
537539
}
538540

541+
// Max concurrent streams check
539542
if let Some(max) = self.0.local_config.remote_max_concurrent_streams {
540543
if self.0.active_remote_streams.get() >= max {
541544
// check if client opened more streams than allowed
@@ -550,6 +553,7 @@ impl RecvHalfConnection {
550553
}
551554
}
552555

556+
// Pseudo-headers validation
553557
let pseudo = frm.pseudo();
554558
if pseudo
555559
.path
@@ -572,6 +576,7 @@ impl RecvHalfConnection {
572576
} else if frm.pseudo().status.is_some() {
573577
Err(Either::Left(ConnectionError::UnexpectedPseudo("scheme")))
574578
} else {
579+
// Create the new stream
575580
let stream = StreamRef::new(id, true, Connection(self.0.clone()));
576581
self.0.streams_count.set(self.0.streams_count.get() + 1);
577582
self.0.streams.borrow_mut().insert(id, stream.clone());

0 commit comments

Comments
 (0)