-
-
Notifications
You must be signed in to change notification settings - Fork 157
support for raw frames in PacketEncoder #691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
f1cf42a
4b0dc6d
ba692aa
e5dde57
7e42392
4a96afa
a4a8918
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,13 +18,13 @@ type Cipher = cfb8::Encryptor<aes::Aes128>; | |
|
|
||
| #[derive(Default)] | ||
| pub struct PacketEncoder { | ||
| buf: BytesMut, | ||
| pub buf: BytesMut, | ||
| #[cfg(feature = "compression")] | ||
| compress_buf: Vec<u8>, | ||
| pub compress_buf: Vec<u8>, | ||
| #[cfg(feature = "compression")] | ||
| threshold: CompressionThreshold, | ||
| pub threshold: CompressionThreshold, | ||
| #[cfg(feature = "encryption")] | ||
| cipher: Option<Cipher>, | ||
| pub cipher: Option<Cipher>, | ||
| } | ||
|
|
||
| impl PacketEncoder { | ||
|
|
@@ -37,37 +37,12 @@ impl PacketEncoder { | |
| self.buf.extend_from_slice(bytes) | ||
| } | ||
|
|
||
| pub fn prepend_packet<P>(&mut self, pkt: &P) -> anyhow::Result<()> | ||
| where | ||
| P: Packet + Encode, | ||
| { | ||
| let start_len = self.buf.len(); | ||
| self.append_packet(pkt)?; | ||
|
|
||
| let end_len = self.buf.len(); | ||
| let total_packet_len = end_len - start_len; | ||
|
|
||
| // 1) Move everything back by the length of the packet. | ||
| // 2) Move the packet to the new space at the front. | ||
| // 3) Truncate the old packet away. | ||
| self.buf.put_bytes(0, total_packet_len); | ||
| self.buf.copy_within(..end_len, total_packet_len); | ||
| self.buf.copy_within(total_packet_len + start_len.., 0); | ||
| self.buf.truncate(end_len); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[allow(clippy::needless_borrows_for_generic_args)] | ||
| pub fn append_packet<P>(&mut self, pkt: &P) -> anyhow::Result<()> | ||
| where | ||
| P: Packet + Encode, | ||
| { | ||
| let start_len = self.buf.len(); | ||
|
|
||
| pkt.encode_with_id((&mut self.buf).writer())?; | ||
|
|
||
| let data_len = self.buf.len() - start_len; | ||
| /// frames the bytes in a range from `from` to the end of the buffer, framing is: | ||
| /// adding a packet length varint to the start of the frame | ||
| /// adding a data length varint after the packet length, if compression is enabled | ||
| /// compressing the packet, if compression is enabled | ||
| pub fn enframe_from(&mut self, from: usize) -> anyhow::Result<()> { | ||
| let data_len = self.buf.len() - from; | ||
|
|
||
| #[cfg(feature = "compression")] | ||
| if self.threshold.0 >= 0 { | ||
|
|
@@ -77,7 +52,7 @@ impl PacketEncoder { | |
| use flate2::Compression; | ||
|
|
||
| if data_len > self.threshold.0 as usize { | ||
| let mut z = ZlibEncoder::new(&self.buf[start_len..], Compression::new(4)); | ||
| let mut z = ZlibEncoder::new(&self.buf[from..], Compression::new(4)); | ||
|
|
||
| self.compress_buf.clear(); | ||
|
|
||
|
|
@@ -92,7 +67,7 @@ impl PacketEncoder { | |
|
|
||
| drop(z); | ||
|
|
||
| self.buf.truncate(start_len); | ||
| self.buf.truncate(from); | ||
|
|
||
| let mut writer = (&mut self.buf).writer(); | ||
|
|
||
|
|
@@ -114,9 +89,9 @@ impl PacketEncoder { | |
|
|
||
| self.buf.put_bytes(0, data_prefix_len); | ||
| self.buf | ||
| .copy_within(start_len..start_len + data_len, start_len + data_prefix_len); | ||
| .copy_within(from..from + data_len, from + data_prefix_len); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I don't like the arbitrary var name changes. IMO this makes the code less readable, and the extra diff makes harder to review.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, maybe i was too lazy with this, this is looks like this bc i didnt interpreted the code chunk that compresses the packet
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think the new names for the vars make sense, because "start_len" was used for the length of the buffer before pushing the frame's bytes, but now it makes sense that its named "from", i changed the var name because i felt that "start_len" didnt maked that much sense for the argument's var name |
||
|
|
||
| let mut front = &mut self.buf[start_len..]; | ||
| let mut front = &mut self.buf[from..]; | ||
|
|
||
| VarInt(packet_len as i32).encode(&mut front)?; | ||
| // Zero for no compression on this packet. | ||
|
|
@@ -137,14 +112,74 @@ impl PacketEncoder { | |
|
|
||
| self.buf.put_bytes(0, packet_len_size); | ||
| self.buf | ||
| .copy_within(start_len..start_len + data_len, start_len + packet_len_size); | ||
| .copy_within(from..from + data_len, from + packet_len_size); | ||
|
|
||
| let front = &mut self.buf[start_len..]; | ||
| let front = &mut self.buf[from..]; | ||
| VarInt(packet_len as i32).encode(front)?; | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn prepend_frame<P>(&mut self, frame: &[u8]) -> anyhow::Result<()> | ||
| where | ||
| P: Packet + Encode, | ||
| { | ||
| let start_len = self.buf.len(); | ||
| self.append_frame(frame)?; | ||
|
|
||
| let end_len = self.buf.len(); | ||
| let total_packet_len = end_len - start_len; | ||
|
|
||
| // 1) Move everything back by the length of the packet. | ||
| // 2) Move the packet to the new space at the front. | ||
| // 3) Truncate the old packet away. | ||
| self.buf.put_bytes(0, total_packet_len); | ||
| self.buf.copy_within(..end_len, total_packet_len); | ||
| self.buf.copy_within(total_packet_len + start_len.., 0); | ||
| self.buf.truncate(end_len); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn prepend_packet<P>(&mut self, pkt: &P) -> anyhow::Result<()> | ||
| where | ||
| P: Packet + Encode, | ||
| { | ||
| let start_len = self.buf.len(); | ||
| self.append_packet(pkt)?; | ||
|
|
||
| let end_len = self.buf.len(); | ||
| let total_packet_len = end_len - start_len; | ||
|
|
||
| // 1) Move everything back by the length of the packet. | ||
| // 2) Move the packet to the new space at the front. | ||
| // 3) Truncate the old packet away. | ||
| self.buf.put_bytes(0, total_packet_len); | ||
| self.buf.copy_within(..end_len, total_packet_len); | ||
| self.buf.copy_within(total_packet_len + start_len.., 0); | ||
| self.buf.truncate(end_len); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn append_frame(&mut self, frame: &[u8]) -> anyhow::Result<()> { | ||
| let start_len = self.buf.len(); | ||
| self.append_bytes(frame); | ||
| self.enframe_from(start_len)?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[allow(clippy::needless_borrows_for_generic_args)] | ||
| pub fn append_packet<P>(&mut self, pkt: &P) -> anyhow::Result<()> | ||
| where | ||
| P: Packet + Encode, | ||
| { | ||
| let start_len = self.buf.len(); | ||
| pkt.encode_with_id((&mut self.buf).writer())?; | ||
| self.enframe_from(start_len)?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Takes all the packets written so far and encrypts them if encryption is | ||
| /// enabled. | ||
| pub fn take(&mut self) -> BytesMut { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "Framing" is not common terminology. It's either obfuscating a concept that can be described in more simple terms, or its adding just adding jargon to something that's already complex.