Skip to content

Commit

Permalink
Specialize extension payload encoding in Unparsed
Browse files Browse the repository at this point in the history
Extension payload, after the RFC4251-encoded name string, is an opaque
blob, not an RFC4251 array of bytes, so it MUST NOT have a u32 length
header.

This had been handled in the `<Extension as Encode>::encode`
implementation, but that led to a bug in which there was a mismatch
between the number of bytes written and the length calculated. This
commit consolidates all the special-case handling of the opaque blob
into `impl Encode for Unparsed`. Any messages (i.e. Extension) that
contain `Unparsed` fields can now just call `encoded_len()` and
`encode()` like for any other field type.

Signed-off-by: Ross Williams <[email protected]>
  • Loading branch information
overhacked committed May 7, 2024
1 parent 1224627 commit 88c8022
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions src/proto/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,10 +622,7 @@ impl Encode for Extension {

fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> {
self.name.encode(writer)?;

// NOTE: extension messages do not contain a length,
// as the inner Vec<u8> will be encoded with it's own length.
writer.write(&self.details.0[..])?;
self.details.encode(writer)?;
Ok(())
}
}
Expand Down Expand Up @@ -657,7 +654,12 @@ impl Encode for Unparsed {
}

fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> {
self.0.encode(writer)
// NOTE: Unparsed fields do not embed a length u32,
// as the inner Vec<u8> encoding is implementation-defined
// (usually an Extension)
writer.write(&self.0[..])?;

Ok(())
}
}

Expand Down

0 comments on commit 88c8022

Please sign in to comment.