Skip to content

Commit 86cbbb7

Browse files
authored
sctp: Make most chunk handling errors non-fatal (#716)
Some parts of the SCTP spec isn't fully implemented, which can lead to spurious errors during chunk processing (e.g. `ErrHandleInitState`). To compensate, pion treats most chunk handling errors as non-fatal. This change adopts pion's behavior, and also fixes `ErrChunkTypeUnhandled` to only ignore the rest of the SCTP packet, as stated in section 3.2. of the spec.
1 parent 37f0e0e commit 86cbbb7

File tree

1 file changed

+14
-5
lines changed

1 file changed

+14
-5
lines changed

sctp/src/association/association_internal.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,15 @@ impl AssociationInternal {
354354
self.handle_chunk_start();
355355

356356
for c in &p.chunks {
357-
self.handle_chunk(&p, c).await?;
357+
match self.handle_chunk(&p, c).await {
358+
Err(Error::ErrChunk) => return Err(Error::ErrChunk),
359+
// stop processing this SCTP packet, discard the unrecognized
360+
// chunk and all further chunks
361+
Err(Error::ErrChunkTypeUnhandled) => break,
362+
// log and continue, the only condition that is fatal is a ABORT chunk
363+
Err(err) => log::warn!("[{}] failed to handle chunk: {}", self.name, err),
364+
Ok(()) => (),
365+
};
358366
}
359367

360368
self.handle_chunk_end();
@@ -645,7 +653,7 @@ impl AssociationInternal {
645653
&& state != AssociationState::CookieWait
646654
&& state != AssociationState::CookieEchoed
647655
{
648-
log::error!("[{}] chunkInit received in state '{}'", self.name, state);
656+
log::warn!("[{}] chunkInit received in state '{}'", self.name, state);
649657
// 5.2.2. Unexpected INIT in States Other than CLOSED, COOKIE-ECHOED,
650658
// COOKIE-WAIT, and SHUTDOWN-ACK-SENT
651659
return Err(Error::ErrHandleInitState);
@@ -2170,10 +2178,11 @@ impl AssociationInternal {
21702178
} else {
21712179
self.handle_init(p, c).await?
21722180
}
2173-
} else if chunk_any.downcast_ref::<ChunkAbort>().is_some()
2174-
|| chunk_any.downcast_ref::<ChunkError>().is_some()
2175-
{
2181+
} else if chunk_any.downcast_ref::<ChunkAbort>().is_some() {
21762182
return Err(Error::ErrChunk);
2183+
} else if let Some(c) = chunk_any.downcast_ref::<ChunkError>() {
2184+
log::error!("[{}] error chunk, with following errors: {}", self.name, c);
2185+
vec![]
21772186
} else if let Some(c) = chunk_any.downcast_ref::<ChunkHeartbeat>() {
21782187
self.handle_heartbeat(c).await?
21792188
} else if let Some(c) = chunk_any.downcast_ref::<ChunkCookieEcho>() {

0 commit comments

Comments
 (0)