From c5ac8a327d620a63298f4794d87f507c8db69187 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 14 Oct 2024 02:57:38 +0200 Subject: [PATCH 01/13] Encode request PDU directly into buffer No temporary allocation of a separate byte buffer needed. --- src/codec/mod.rs | 199 ++++++++++++++++++++++------------------------- src/codec/rtu.rs | 14 ++-- src/codec/tcp.rs | 18 +++-- 3 files changed, 114 insertions(+), 117 deletions(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 41f3a72..6dfd825 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -19,6 +19,7 @@ pub(crate) mod rtu; #[cfg(feature = "tcp")] pub(crate) mod tcp; +#[cfg(any(test, feature = "rtu", feature = "tcp"))] #[allow(clippy::cast_possible_truncation)] fn u16_len(len: usize) -> u16 { // This type conversion should always be safe, because either @@ -37,82 +38,63 @@ fn u8_len(len: usize) -> u8 { len as u8 } -impl<'a> TryFrom> for Bytes { - type Error = Error; - - #[allow(clippy::panic_in_result_fn)] // Intentional unreachable!() - fn try_from(req: Request<'a>) -> Result { - use crate::frame::Request::*; - let cnt = request_byte_count(&req); - let mut data = BytesMut::with_capacity(cnt); - data.put_u8(req.function_code().value()); - match req { - ReadCoils(address, quantity) - | ReadDiscreteInputs(address, quantity) - | ReadInputRegisters(address, quantity) - | ReadHoldingRegisters(address, quantity) => { - data.put_u16(address); - data.put_u16(quantity); - } - WriteSingleCoil(address, state) => { - data.put_u16(address); - data.put_u16(bool_to_coil(state)); - } - WriteMultipleCoils(address, coils) => { - data.put_u16(address); - let len = coils.len(); - data.put_u16(u16_len(len)); - let packed_coils = pack_coils(&coils); - data.put_u8(u8_len(packed_coils.len())); - for b in packed_coils { - data.put_u8(b); - } - } - WriteSingleRegister(address, word) => { - data.put_u16(address); - data.put_u16(word); - } - WriteMultipleRegisters(address, words) => { - data.put_u16(address); - let len = words.len(); - data.put_u16(u16_len(len)); - data.put_u8(u8_len(len * 2)); - for w in &*words { - data.put_u16(*w); - } - } - ReportServerId => {} - MaskWriteRegister(address, and_mask, or_mask) => { - data.put_u16(address); - data.put_u16(and_mask); - data.put_u16(or_mask); - } - ReadWriteMultipleRegisters(read_address, quantity, write_address, words) => { - data.put_u16(read_address); - data.put_u16(quantity); - data.put_u16(write_address); - let n = words.len(); - data.put_u16(u16_len(n)); - data.put_u8(u8_len(n * 2)); - for w in &*words { - data.put_u16(*w); - } +#[cfg(any(test, feature = "rtu", feature = "tcp"))] +fn encode_request_pdu(buf: &mut BytesMut, request: &Request<'_>) { + use crate::frame::Request::*; + buf.put_u8(request.function_code().value()); + match request { + ReadCoils(address, quantity) + | ReadDiscreteInputs(address, quantity) + | ReadInputRegisters(address, quantity) + | ReadHoldingRegisters(address, quantity) => { + buf.put_u16(*address); + buf.put_u16(*quantity); + } + WriteSingleCoil(address, state) => { + buf.put_u16(*address); + buf.put_u16(bool_to_coil(*state)); + } + WriteMultipleCoils(address, coils) => { + buf.put_u16(*address); + let len = coils.len(); + buf.put_u16(u16_len(len)); + let packed_coils = pack_coils(coils); + buf.put_u8(u8_len(packed_coils.len())); + buf.put_slice(&packed_coils); + } + WriteSingleRegister(address, word) => { + buf.put_u16(*address); + buf.put_u16(*word); + } + WriteMultipleRegisters(address, words) => { + buf.put_u16(*address); + let len = words.len(); + buf.put_u16(u16_len(len)); + buf.put_u8(u8_len(len * 2)); + for w in words.as_ref() { + buf.put_u16(*w); } - Custom(_, custom_data) => { - for d in &*custom_data { - data.put_u8(*d); - } + } + ReportServerId => {} + MaskWriteRegister(address, and_mask, or_mask) => { + buf.put_u16(*address); + buf.put_u16(*and_mask); + buf.put_u16(*or_mask); + } + ReadWriteMultipleRegisters(read_address, quantity, write_address, words) => { + buf.put_u16(*read_address); + buf.put_u16(*quantity); + buf.put_u16(*write_address); + let n = words.len(); + buf.put_u16(u16_len(n)); + buf.put_u8(u8_len(n * 2)); + for w in words.as_ref() { + buf.put_u16(*w); } } - Ok(data.freeze()) - } -} - -impl<'a> TryFrom> for Bytes { - type Error = Error; - - fn try_from(pdu: RequestPdu<'a>) -> Result { - pdu.0.try_into() + Custom(_, custom_data) => { + buf.put_slice(custom_data.as_ref()); + } } } @@ -444,7 +426,8 @@ fn unpack_coils(bytes: &[u8], count: u16) -> Vec { res } -fn request_byte_count(req: &Request<'_>) -> usize { +#[cfg(any(feature = "rtu", feature = "tcp"))] +fn request_pdu_size(req: &Request<'_>) -> usize { use crate::frame::Request::*; match *req { ReadCoils(_, _) @@ -486,6 +469,12 @@ mod tests { use super::*; + fn request_to_pdu_bytes(request: &Request<'_>) -> Bytes { + let mut buf = BytesMut::new(); + super::encode_request_pdu(&mut buf, request); + buf.freeze() + } + #[test] fn convert_bool_to_coil() { assert_eq!(bool_to_coil(true), 0xFF00); @@ -551,7 +540,7 @@ mod tests { #[test] fn pdu_into_bytes() { - let req_pdu: Bytes = Request::ReadCoils(0x01, 5).try_into().unwrap(); + let req_pdu: Bytes = request_to_pdu_bytes(&Request::ReadCoils(0x01, 5)); let rsp_pdu: Bytes = Response::ReadCoils(vec![]).into(); let ex_pdu: Bytes = ExceptionResponse { function: FunctionCode::ReadHoldingRegisters, @@ -571,7 +560,7 @@ mod tests { assert_eq!(ex_pdu[0], 0x83); assert_eq!(ex_pdu[1], 0x04); - let req_pdu: Bytes = Request::ReadHoldingRegisters(0x082B, 2).try_into().unwrap(); + let req_pdu: Bytes = request_to_pdu_bytes(&Request::ReadHoldingRegisters(0x082B, 2)); assert_eq!(req_pdu.len(), 5); assert_eq!(req_pdu[0], 0x03); assert_eq!(req_pdu[1], 0x08); @@ -582,9 +571,10 @@ mod tests { #[test] fn pdu_with_a_lot_of_data_into_bytes() { - let _req_pdu: Bytes = Request::WriteMultipleRegisters(0x01, Cow::Borrowed(&[0; 80])) - .try_into() - .unwrap(); + let _req_pdu: Bytes = request_to_pdu_bytes(&Request::WriteMultipleRegisters( + 0x01, + Cow::Borrowed(&[0; 80]), + )); let _rsp_pdu: Bytes = Response::ReadInputRegisters(vec![0; 80]).into(); } @@ -594,7 +584,7 @@ mod tests { #[test] fn read_coils() { - let bytes: Bytes = Request::ReadCoils(0x12, 4).try_into().unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::ReadCoils(0x12, 4)); assert_eq!(bytes[0], 1); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x12); @@ -604,7 +594,7 @@ mod tests { #[test] fn read_discrete_inputs() { - let bytes: Bytes = Request::ReadDiscreteInputs(0x03, 19).try_into().unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::ReadDiscreteInputs(0x03, 19)); assert_eq!(bytes[0], 2); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x03); @@ -614,7 +604,7 @@ mod tests { #[test] fn write_single_coil() { - let bytes: Bytes = Request::WriteSingleCoil(0x1234, true).try_into().unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::WriteSingleCoil(0x1234, true)); assert_eq!(bytes[0], 5); assert_eq!(bytes[1], 0x12); assert_eq!(bytes[2], 0x34); @@ -625,9 +615,8 @@ mod tests { #[test] fn write_multiple_coils() { let states = [true, false, true, true]; - let bytes: Bytes = Request::WriteMultipleCoils(0x3311, Cow::Borrowed(&states)) - .try_into() - .unwrap(); + let bytes: Bytes = + request_to_pdu_bytes(&Request::WriteMultipleCoils(0x3311, Cow::Borrowed(&states))); assert_eq!(bytes[0], 0x0F); assert_eq!(bytes[1], 0x33); assert_eq!(bytes[2], 0x11); @@ -639,7 +628,7 @@ mod tests { #[test] fn read_input_registers() { - let bytes: Bytes = Request::ReadInputRegisters(0x09, 77).try_into().unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::ReadInputRegisters(0x09, 77)); assert_eq!(bytes[0], 4); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x09); @@ -649,7 +638,7 @@ mod tests { #[test] fn read_holding_registers() { - let bytes: Bytes = Request::ReadHoldingRegisters(0x09, 77).try_into().unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::ReadHoldingRegisters(0x09, 77)); assert_eq!(bytes[0], 3); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x09); @@ -659,9 +648,7 @@ mod tests { #[test] fn write_single_register() { - let bytes: Bytes = Request::WriteSingleRegister(0x07, 0xABCD) - .try_into() - .unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::WriteSingleRegister(0x07, 0xABCD)); assert_eq!(bytes[0], 6); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x07); @@ -671,10 +658,10 @@ mod tests { #[test] fn write_multiple_registers() { - let bytes: Bytes = - Request::WriteMultipleRegisters(0x06, Cow::Borrowed(&[0xABCD, 0xEF12])) - .try_into() - .unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::WriteMultipleRegisters( + 0x06, + Cow::Borrowed(&[0xABCD, 0xEF12]), + )); // function code assert_eq!(bytes[0], 0x10); @@ -699,15 +686,14 @@ mod tests { #[test] fn report_server_id() { - let bytes: Bytes = Request::ReportServerId.try_into().unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::ReportServerId); assert_eq!(bytes[0], 0x11); } #[test] fn masked_write_register() { - let bytes: Bytes = Request::MaskWriteRegister(0xABCD, 0xEF12, 0x2345) - .try_into() - .unwrap(); + let bytes: Bytes = + request_to_pdu_bytes(&Request::MaskWriteRegister(0xABCD, 0xEF12, 0x2345)); // function code assert_eq!(bytes[0], 0x16); @@ -728,10 +714,12 @@ mod tests { #[test] fn read_write_multiple_registers() { let data = [0xABCD, 0xEF12]; - let bytes: Bytes = - Request::ReadWriteMultipleRegisters(0x05, 51, 0x03, Cow::Borrowed(&data)) - .try_into() - .unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::ReadWriteMultipleRegisters( + 0x05, + 51, + 0x03, + Cow::Borrowed(&data), + )); // function code assert_eq!(bytes[0], 0x17); @@ -764,9 +752,10 @@ mod tests { #[test] fn custom() { - let bytes: Bytes = Request::Custom(0x55, Cow::Borrowed(&[0xCC, 0x88, 0xAA, 0xFF])) - .try_into() - .unwrap(); + let bytes: Bytes = request_to_pdu_bytes(&Request::Custom( + 0x55, + Cow::Borrowed(&[0xCC, 0x88, 0xAA, 0xFF]), + )); assert_eq!(bytes[0], 0x55); assert_eq!(bytes[1], 0xCC); assert_eq!(bytes[2], 0x88); diff --git a/src/codec/rtu.rs b/src/codec/rtu.rs index 1978fd3..7a5450c 100644 --- a/src/codec/rtu.rs +++ b/src/codec/rtu.rs @@ -328,12 +328,16 @@ impl<'a> Encoder> for ClientCodec { type Error = Error; fn encode(&mut self, adu: RequestAdu<'a>, buf: &mut BytesMut) -> Result<()> { - let RequestAdu { hdr, pdu } = adu; - let pdu_data: Bytes = pdu.try_into()?; - buf.reserve(pdu_data.len() + 3); + let RequestAdu { + hdr, + pdu: RequestPdu(request), + } = adu; + let buf_offset = buf.len(); + let request_pdu_size = request_pdu_size(&request); + buf.reserve((buf.capacity() - buf_offset) + request_pdu_size + 3); buf.put_u8(hdr.slave_id); - buf.put_slice(&pdu_data); - let crc = calc_crc(buf); + encode_request_pdu(buf, &request); + let crc = calc_crc(&buf[buf_offset..]); buf.put_u16(crc); Ok(()) } diff --git a/src/codec/tcp.rs b/src/codec/tcp.rs index 05fb88e..eb9a2ce 100644 --- a/src/codec/tcp.rs +++ b/src/codec/tcp.rs @@ -127,14 +127,18 @@ impl<'a> Encoder> for ClientCodec { type Error = Error; fn encode(&mut self, adu: RequestAdu<'a>, buf: &mut BytesMut) -> Result<()> { - let RequestAdu { hdr, pdu } = adu; - let pdu_data: Bytes = pdu.try_into()?; - buf.reserve(pdu_data.len() + 7); + let RequestAdu { + hdr, + pdu: RequestPdu(request), + } = adu; + let buf_offset = buf.len(); + let request_pdu_size = request_pdu_size(&request); + buf.reserve((buf.capacity() - buf_offset) + request_pdu_size + 7); buf.put_u16(hdr.transaction_id); buf.put_u16(PROTOCOL_ID); - buf.put_u16(u16_len(pdu_data.len() + 1)); + buf.put_u16(u16_len(request_pdu_size + 1)); buf.put_u8(hdr.unit_id); - buf.put_slice(&pdu_data); + encode_request_pdu(buf, &request); Ok(()) } } @@ -159,7 +163,6 @@ impl Encoder for ServerCodec { #[cfg(test)] mod tests { use super::*; - use crate::bytes::Bytes; mod client { @@ -274,7 +277,8 @@ mod tests { assert_eq!(buf[6], UNIT_ID); drop(buf.split_to(7)); - let pdu: Bytes = req.try_into().unwrap(); + let mut pdu = BytesMut::new(); + encode_request_pdu(&mut pdu, &req); assert_eq!(buf, pdu); } From 9f0379bd1ba20f9296353ed5c5c053c80394e9cd Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 14 Oct 2024 02:53:07 +0200 Subject: [PATCH 02/13] Pack coils directly into buffer --- src/codec/mod.rs | 63 ++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 6dfd825..cf169de 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -56,11 +56,9 @@ fn encode_request_pdu(buf: &mut BytesMut, request: &Request<'_>) { } WriteMultipleCoils(address, coils) => { buf.put_u16(*address); - let len = coils.len(); - buf.put_u16(u16_len(len)); - let packed_coils = pack_coils(coils); - buf.put_u8(u8_len(packed_coils.len())); - buf.put_slice(&packed_coils); + buf.put_u16(u16_len(coils.len())); + buf.put_u8(u8_len(packed_coils_len(coils.len()))); + pack_coils(buf, coils); } WriteSingleRegister(address, word) => { buf.put_u16(*address); @@ -106,11 +104,8 @@ impl From for Bytes { data.put_u8(rsp.function_code().value()); match rsp { ReadCoils(coils) | ReadDiscreteInputs(coils) => { - let packed_coils = pack_coils(&coils); - data.put_u8(u8_len(packed_coils.len())); - for b in packed_coils { - data.put_u8(b); - } + data.put_u8(u8_len(packed_coils_len(coils.len()))); + pack_coils(&mut data, &coils); } ReadInputRegisters(registers) | ReadHoldingRegisters(registers) @@ -132,9 +127,7 @@ impl From for Bytes { data.put_u8(2 + u8_len(additional_data.len())); data.put_u8(server_id); data.put_u8(if run_indication { 0xFF } else { 0x00 }); - for b in additional_data { - data.put_u8(b); - } + data.put_slice(&additional_data); } WriteSingleRegister(address, word) => { data.put_u16(address); @@ -146,9 +139,7 @@ impl From for Bytes { data.put_u16(or_mask); } Custom(_, custom_data) => { - for d in custom_data { - data.put_u8(d); - } + data.put_slice(&custom_data); } } data.freeze() @@ -408,14 +399,16 @@ fn packed_coils_len(bitcount: usize) -> usize { (bitcount + 7) / 8 } -fn pack_coils(coils: &[Coil]) -> Vec { - let packed_size = packed_coils_len(coils.len()); - let mut res = vec![0; packed_size]; +fn pack_coils(buf: &mut BytesMut, coils: &[Coil]) -> usize { + let packed_coils_len = packed_coils_len(coils.len()); + let offset = buf.len(); + buf.resize(offset + packed_coils_len, 0); + let buf = &mut buf[offset..]; for (i, b) in coils.iter().enumerate() { let v = u8::from(*b); // 0 or 1 - res[i / 8] |= v << (i % 8); + buf[i / 8] |= v << (i % 8); } - res + packed_coils_len } fn unpack_coils(bytes: &[u8], count: u16) -> Vec { @@ -471,7 +464,13 @@ mod tests { fn request_to_pdu_bytes(request: &Request<'_>) -> Bytes { let mut buf = BytesMut::new(); - super::encode_request_pdu(&mut buf, request); + encode_request_pdu(&mut buf, request); + buf.freeze() + } + + fn pack_coils_to_bytes(coils: &[bool]) -> Bytes { + let mut buf = BytesMut::new(); + pack_coils(&mut buf, coils); buf.freeze() } @@ -489,16 +488,16 @@ mod tests { #[test] fn convert_booleans_to_bytes() { - assert_eq!(pack_coils(&[]), &[]); - assert_eq!(pack_coils(&[true]), &[0b1]); - assert_eq!(pack_coils(&[false]), &[0b0]); - assert_eq!(pack_coils(&[true, false]), &[0b_01]); - assert_eq!(pack_coils(&[false, true]), &[0b_10]); - assert_eq!(pack_coils(&[true, true]), &[0b_11]); - assert_eq!(pack_coils(&[true; 8]), &[0b_1111_1111]); - assert_eq!(pack_coils(&[true; 9]), &[255, 1]); - assert_eq!(pack_coils(&[false; 8]), &[0]); - assert_eq!(pack_coils(&[false; 9]), &[0, 0]); + assert_eq!(&pack_coils_to_bytes(&[])[..], &[]); + assert_eq!(&pack_coils_to_bytes(&[true])[..], &[0b1]); + assert_eq!(&pack_coils_to_bytes(&[false])[..], &[0b0]); + assert_eq!(&pack_coils_to_bytes(&[true, false])[..], &[0b_01]); + assert_eq!(&pack_coils_to_bytes(&[false, true])[..], &[0b_10]); + assert_eq!(&pack_coils_to_bytes(&[true, true])[..], &[0b_11]); + assert_eq!(&pack_coils_to_bytes(&[true; 8])[..], &[0b_1111_1111]); + assert_eq!(&pack_coils_to_bytes(&[true; 9])[..], &[255, 1]); + assert_eq!(&pack_coils_to_bytes(&[false; 8])[..], &[0]); + assert_eq!(&pack_coils_to_bytes(&[false; 9])[..], &[0, 0]); } #[test] From a618bb65c5f6c28587ad6316cee73c91d70883a8 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 14 Oct 2024 10:28:30 +0200 Subject: [PATCH 03/13] Encode response PDU directly into buffer --- src/codec/mod.rs | 305 +++++++++++++++++++++++++++-------------------- src/codec/rtu.rs | 23 ++-- src/codec/tcp.rs | 13 +- 3 files changed, 197 insertions(+), 144 deletions(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index cf169de..8549861 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -9,8 +9,9 @@ use std::{ use byteorder::{BigEndian, ReadBytesExt as _}; use crate::{ - bytes::{BufMut, Bytes, BytesMut}, - frame::*, + bytes::Bytes, + frame::{Coil, RequestPdu, ResponsePdu}, + ExceptionCode, ExceptionResponse, FunctionCode, Request, Response, }; #[cfg(feature = "rtu")] @@ -29,6 +30,7 @@ fn u16_len(len: usize) -> u16 { len as u16 } +#[cfg(any(test, feature = "rtu", feature = "tcp"))] #[allow(clippy::cast_possible_truncation)] fn u8_len(len: usize) -> u8 { // This type conversion should always be safe, because either @@ -39,8 +41,8 @@ fn u8_len(len: usize) -> u8 { } #[cfg(any(test, feature = "rtu", feature = "tcp"))] -fn encode_request_pdu(buf: &mut BytesMut, request: &Request<'_>) { - use crate::frame::Request::*; +fn encode_request_pdu(buf: &mut crate::bytes::BytesMut, request: &Request<'_>) { + use crate::{bytes::BufMut as _, frame::Request::*}; buf.put_u8(request.function_code().value()); match request { ReadCoils(address, quantity) @@ -57,8 +59,8 @@ fn encode_request_pdu(buf: &mut BytesMut, request: &Request<'_>) { WriteMultipleCoils(address, coils) => { buf.put_u16(*address); buf.put_u16(u16_len(coils.len())); - buf.put_u8(u8_len(packed_coils_len(coils.len()))); - pack_coils(buf, coils); + buf.put_u8(u8_len(packed_coils_size(coils))); + encode_packed_coils(buf, coils); } WriteSingleRegister(address, word) => { buf.put_u16(*address); @@ -96,69 +98,68 @@ fn encode_request_pdu(buf: &mut BytesMut, request: &Request<'_>) { } } -impl From for Bytes { - fn from(rsp: Response) -> Bytes { - use crate::frame::Response::*; - let cnt = response_byte_count(&rsp); - let mut data = BytesMut::with_capacity(cnt); - data.put_u8(rsp.function_code().value()); - match rsp { - ReadCoils(coils) | ReadDiscreteInputs(coils) => { - data.put_u8(u8_len(packed_coils_len(coils.len()))); - pack_coils(&mut data, &coils); - } - ReadInputRegisters(registers) - | ReadHoldingRegisters(registers) - | ReadWriteMultipleRegisters(registers) => { - data.put_u8(u8_len(registers.len() * 2)); - for r in registers { - data.put_u16(r); - } - } - WriteSingleCoil(address, state) => { - data.put_u16(address); - data.put_u16(bool_to_coil(state)); - } - WriteMultipleCoils(address, quantity) | WriteMultipleRegisters(address, quantity) => { - data.put_u16(address); - data.put_u16(quantity); - } - ReportServerId(server_id, run_indication, additional_data) => { - data.put_u8(2 + u8_len(additional_data.len())); - data.put_u8(server_id); - data.put_u8(if run_indication { 0xFF } else { 0x00 }); - data.put_slice(&additional_data); - } - WriteSingleRegister(address, word) => { - data.put_u16(address); - data.put_u16(word); - } - MaskWriteRegister(address, and_mask, or_mask) => { - data.put_u16(address); - data.put_u16(and_mask); - data.put_u16(or_mask); - } - Custom(_, custom_data) => { - data.put_slice(&custom_data); +#[cfg(any(test, feature = "server"))] +fn encode_response_pdu(buf: &mut crate::bytes::BytesMut, rsp: &Response) { + use crate::{bytes::BufMut as _, frame::Response::*}; + buf.put_u8(rsp.function_code().value()); + match rsp { + ReadCoils(coils) | ReadDiscreteInputs(coils) => { + buf.put_u8(u8_len(packed_coils_size(coils))); + encode_packed_coils(buf, coils); + } + ReadInputRegisters(registers) + | ReadHoldingRegisters(registers) + | ReadWriteMultipleRegisters(registers) => { + buf.put_u8(u8_len(registers.len() * 2)); + for r in registers { + buf.put_u16(*r); } } - data.freeze() + WriteSingleCoil(address, state) => { + buf.put_u16(*address); + buf.put_u16(bool_to_coil(*state)); + } + WriteMultipleCoils(address, quantity) | WriteMultipleRegisters(address, quantity) => { + buf.put_u16(*address); + buf.put_u16(*quantity); + } + ReportServerId(server_id, run_indication, additional_data) => { + buf.put_u8(2 + u8_len(additional_data.len())); + buf.put_u8(*server_id); + buf.put_u8(if *run_indication { 0xFF } else { 0x00 }); + buf.put_slice(additional_data); + } + WriteSingleRegister(address, word) => { + buf.put_u16(*address); + buf.put_u16(*word); + } + MaskWriteRegister(address, and_mask, or_mask) => { + buf.put_u16(*address); + buf.put_u16(*and_mask); + buf.put_u16(*or_mask); + } + Custom(_, custom_data) => { + buf.put_slice(custom_data); + } } } -impl From for Bytes { - fn from(ex: ExceptionResponse) -> Bytes { - let mut data = BytesMut::with_capacity(2); - debug_assert!(ex.function.value() < 0x80); - data.put_u8(ex.function.value() + 0x80); - data.put_u8(ex.exception.into()); - data.freeze() - } +#[cfg(any(test, feature = "server"))] +fn encode_exception_response_pdu(buf: &mut crate::bytes::BytesMut, rsp: ExceptionResponse) { + use crate::bytes::BufMut as _; + debug_assert!(rsp.function.value() < 0x80); + buf.put_u8(rsp.function.value() + 0x80); + buf.put_u8(rsp.exception.into()); } -impl From for Bytes { - fn from(pdu: ResponsePdu) -> Bytes { - pdu.0.map_or_else(Into::into, Into::into) +#[cfg(feature = "server")] +fn encode_response_result_pdu( + buf: &mut crate::bytes::BytesMut, + res: &Result, +) { + match res { + Ok(rsp) => encode_response_pdu(buf, rsp), + Err(rsp) => encode_exception_response_pdu(buf, *rsp), } } @@ -379,6 +380,7 @@ impl TryFrom for ResponsePdu { } } +#[cfg(any(test, feature = "rtu", feature = "tcp"))] fn bool_to_coil(state: bool) -> u16 { if state { 0xFF00 @@ -395,20 +397,22 @@ fn coil_to_bool(coil: u16) -> io::Result { } } -fn packed_coils_len(bitcount: usize) -> usize { - (bitcount + 7) / 8 +#[cfg(any(test, feature = "rtu", feature = "tcp"))] +fn packed_coils_size(coils: &[Coil]) -> usize { + (coils.len() + 7) / 8 } -fn pack_coils(buf: &mut BytesMut, coils: &[Coil]) -> usize { - let packed_coils_len = packed_coils_len(coils.len()); +#[cfg(any(test, feature = "rtu", feature = "tcp"))] +fn encode_packed_coils(buf: &mut crate::bytes::BytesMut, coils: &[Coil]) -> usize { + let packed_coils_size = packed_coils_size(coils); let offset = buf.len(); - buf.resize(offset + packed_coils_len, 0); + buf.resize(offset + packed_coils_size, 0); let buf = &mut buf[offset..]; for (i, b) in coils.iter().enumerate() { let v = u8::from(*b); // 0 or 1 buf[i / 8] |= v << (i % 8); } - packed_coils_len + packed_coils_size } fn unpack_coils(bytes: &[u8], count: u16) -> Vec { @@ -422,55 +426,78 @@ fn unpack_coils(bytes: &[u8], count: u16) -> Vec { #[cfg(any(feature = "rtu", feature = "tcp"))] fn request_pdu_size(req: &Request<'_>) -> usize { use crate::frame::Request::*; - match *req { + match req { ReadCoils(_, _) | ReadDiscreteInputs(_, _) | ReadInputRegisters(_, _) | ReadHoldingRegisters(_, _) | WriteSingleRegister(_, _) | WriteSingleCoil(_, _) => 5, - WriteMultipleCoils(_, ref coils) => 6 + packed_coils_len(coils.len()), - WriteMultipleRegisters(_, ref data) => 6 + data.len() * 2, + WriteMultipleCoils(_, coils) => 6 + packed_coils_size(coils), + WriteMultipleRegisters(_, data) => 6 + data.len() * 2, ReportServerId => 1, MaskWriteRegister(_, _, _) => 7, - ReadWriteMultipleRegisters(_, _, _, ref data) => 10 + data.len() * 2, - Custom(_, ref data) => 1 + data.len(), + ReadWriteMultipleRegisters(_, _, _, data) => 10 + data.len() * 2, + Custom(_, data) => 1 + data.len(), } } -fn response_byte_count(rsp: &Response) -> usize { +#[cfg(feature = "server")] +fn response_pdu_size(rsp: &Response) -> usize { use crate::frame::Response::*; - match *rsp { - ReadCoils(ref coils) | ReadDiscreteInputs(ref coils) => 2 + packed_coils_len(coils.len()), + match rsp { + ReadCoils(coils) | ReadDiscreteInputs(coils) => 2 + packed_coils_size(coils), WriteSingleCoil(_, _) | WriteMultipleCoils(_, _) | WriteMultipleRegisters(_, _) | WriteSingleRegister(_, _) => 5, - ReadInputRegisters(ref data) - | ReadHoldingRegisters(ref data) - | ReadWriteMultipleRegisters(ref data) => 2 + data.len() * 2, + ReadInputRegisters(data) + | ReadHoldingRegisters(data) + | ReadWriteMultipleRegisters(data) => 2 + data.len() * 2, ReportServerId(_, _, ref data) => 3 + data.len(), MaskWriteRegister(_, _, _) => 7, Custom(_, ref data) => 1 + data.len(), } } +#[cfg(feature = "server")] +fn response_result_pdu_size(res: &Result) -> usize { + match res { + Ok(rsp) => response_pdu_size(rsp), + Err(_) => 2, + } +} + #[cfg(test)] mod tests { use std::borrow::Cow; + use crate::bytes::BytesMut; + use super::*; - fn request_to_pdu_bytes(request: &Request<'_>) -> Bytes { + fn encode_request_pdu_to_bytes(request: &Request<'_>) -> Bytes { let mut buf = BytesMut::new(); encode_request_pdu(&mut buf, request); buf.freeze() } - fn pack_coils_to_bytes(coils: &[bool]) -> Bytes { + fn encode_response_pdu_to_bytes(response: &Response) -> Bytes { + let mut buf = BytesMut::new(); + encode_response_pdu(&mut buf, response); + buf.freeze() + } + + fn encode_exception_response_pdu_to_bytes(response: ExceptionResponse) -> Bytes { let mut buf = BytesMut::new(); - pack_coils(&mut buf, coils); + encode_exception_response_pdu(&mut buf, response); + buf.freeze() + } + + fn encode_packed_coils_to_bytes(coils: &[Coil]) -> Bytes { + let mut buf = BytesMut::new(); + encode_packed_coils(&mut buf, coils); buf.freeze() } @@ -488,16 +515,19 @@ mod tests { #[test] fn convert_booleans_to_bytes() { - assert_eq!(&pack_coils_to_bytes(&[])[..], &[]); - assert_eq!(&pack_coils_to_bytes(&[true])[..], &[0b1]); - assert_eq!(&pack_coils_to_bytes(&[false])[..], &[0b0]); - assert_eq!(&pack_coils_to_bytes(&[true, false])[..], &[0b_01]); - assert_eq!(&pack_coils_to_bytes(&[false, true])[..], &[0b_10]); - assert_eq!(&pack_coils_to_bytes(&[true, true])[..], &[0b_11]); - assert_eq!(&pack_coils_to_bytes(&[true; 8])[..], &[0b_1111_1111]); - assert_eq!(&pack_coils_to_bytes(&[true; 9])[..], &[255, 1]); - assert_eq!(&pack_coils_to_bytes(&[false; 8])[..], &[0]); - assert_eq!(&pack_coils_to_bytes(&[false; 9])[..], &[0, 0]); + assert_eq!(&encode_packed_coils_to_bytes(&[])[..], &[]); + assert_eq!(&encode_packed_coils_to_bytes(&[true])[..], &[0b1]); + assert_eq!(&encode_packed_coils_to_bytes(&[false])[..], &[0b0]); + assert_eq!(&encode_packed_coils_to_bytes(&[true, false])[..], &[0b_01]); + assert_eq!(&encode_packed_coils_to_bytes(&[false, true])[..], &[0b_10]); + assert_eq!(&encode_packed_coils_to_bytes(&[true, true])[..], &[0b_11]); + assert_eq!( + &encode_packed_coils_to_bytes(&[true; 8])[..], + &[0b_1111_1111] + ); + assert_eq!(&encode_packed_coils_to_bytes(&[true; 9])[..], &[255, 1]); + assert_eq!(&encode_packed_coils_to_bytes(&[false; 8])[..], &[0]); + assert_eq!(&encode_packed_coils_to_bytes(&[false; 9])[..], &[0, 0]); } #[test] @@ -513,11 +543,10 @@ mod tests { #[test] fn exception_response_into_bytes() { - let bytes: Bytes = ExceptionResponse { + let bytes = encode_exception_response_pdu_to_bytes(ExceptionResponse { function: FunctionCode::ReadHoldingRegisters, exception: ExceptionCode::IllegalDataAddress, - } - .into(); + }); assert_eq!(bytes[0], 0x83); assert_eq!(bytes[1], 0x02); } @@ -539,13 +568,12 @@ mod tests { #[test] fn pdu_into_bytes() { - let req_pdu: Bytes = request_to_pdu_bytes(&Request::ReadCoils(0x01, 5)); - let rsp_pdu: Bytes = Response::ReadCoils(vec![]).into(); - let ex_pdu: Bytes = ExceptionResponse { + let req_pdu = encode_request_pdu_to_bytes(&Request::ReadCoils(0x01, 5)); + let rsp_pdu = encode_response_pdu_to_bytes(&Response::ReadCoils(vec![])); + let ex_pdu = encode_exception_response_pdu_to_bytes(ExceptionResponse { function: FunctionCode::ReadHoldingRegisters, exception: ExceptionCode::ServerDeviceFailure, - } - .into(); + }); assert_eq!(req_pdu[0], 0x01); assert_eq!(req_pdu[1], 0x00); @@ -559,7 +587,7 @@ mod tests { assert_eq!(ex_pdu[0], 0x83); assert_eq!(ex_pdu[1], 0x04); - let req_pdu: Bytes = request_to_pdu_bytes(&Request::ReadHoldingRegisters(0x082B, 2)); + let req_pdu = encode_request_pdu_to_bytes(&Request::ReadHoldingRegisters(0x082B, 2)); assert_eq!(req_pdu.len(), 5); assert_eq!(req_pdu[0], 0x03); assert_eq!(req_pdu[1], 0x08); @@ -570,11 +598,11 @@ mod tests { #[test] fn pdu_with_a_lot_of_data_into_bytes() { - let _req_pdu: Bytes = request_to_pdu_bytes(&Request::WriteMultipleRegisters( + let _req_pdu = encode_request_pdu_to_bytes(&Request::WriteMultipleRegisters( 0x01, Cow::Borrowed(&[0; 80]), )); - let _rsp_pdu: Bytes = Response::ReadInputRegisters(vec![0; 80]).into(); + let _rsp_pdu = encode_response_pdu_to_bytes(&Response::ReadInputRegisters(vec![0; 80])); } mod serialize_requests { @@ -583,7 +611,7 @@ mod tests { #[test] fn read_coils() { - let bytes: Bytes = request_to_pdu_bytes(&Request::ReadCoils(0x12, 4)); + let bytes = encode_request_pdu_to_bytes(&Request::ReadCoils(0x12, 4)); assert_eq!(bytes[0], 1); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x12); @@ -593,7 +621,7 @@ mod tests { #[test] fn read_discrete_inputs() { - let bytes: Bytes = request_to_pdu_bytes(&Request::ReadDiscreteInputs(0x03, 19)); + let bytes = encode_request_pdu_to_bytes(&Request::ReadDiscreteInputs(0x03, 19)); assert_eq!(bytes[0], 2); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x03); @@ -603,7 +631,7 @@ mod tests { #[test] fn write_single_coil() { - let bytes: Bytes = request_to_pdu_bytes(&Request::WriteSingleCoil(0x1234, true)); + let bytes = encode_request_pdu_to_bytes(&Request::WriteSingleCoil(0x1234, true)); assert_eq!(bytes[0], 5); assert_eq!(bytes[1], 0x12); assert_eq!(bytes[2], 0x34); @@ -614,8 +642,10 @@ mod tests { #[test] fn write_multiple_coils() { let states = [true, false, true, true]; - let bytes: Bytes = - request_to_pdu_bytes(&Request::WriteMultipleCoils(0x3311, Cow::Borrowed(&states))); + let bytes = encode_request_pdu_to_bytes(&Request::WriteMultipleCoils( + 0x3311, + Cow::Borrowed(&states), + )); assert_eq!(bytes[0], 0x0F); assert_eq!(bytes[1], 0x33); assert_eq!(bytes[2], 0x11); @@ -627,7 +657,7 @@ mod tests { #[test] fn read_input_registers() { - let bytes: Bytes = request_to_pdu_bytes(&Request::ReadInputRegisters(0x09, 77)); + let bytes = encode_request_pdu_to_bytes(&Request::ReadInputRegisters(0x09, 77)); assert_eq!(bytes[0], 4); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x09); @@ -637,7 +667,7 @@ mod tests { #[test] fn read_holding_registers() { - let bytes: Bytes = request_to_pdu_bytes(&Request::ReadHoldingRegisters(0x09, 77)); + let bytes = encode_request_pdu_to_bytes(&Request::ReadHoldingRegisters(0x09, 77)); assert_eq!(bytes[0], 3); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x09); @@ -647,7 +677,7 @@ mod tests { #[test] fn write_single_register() { - let bytes: Bytes = request_to_pdu_bytes(&Request::WriteSingleRegister(0x07, 0xABCD)); + let bytes = encode_request_pdu_to_bytes(&Request::WriteSingleRegister(0x07, 0xABCD)); assert_eq!(bytes[0], 6); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x07); @@ -657,7 +687,7 @@ mod tests { #[test] fn write_multiple_registers() { - let bytes: Bytes = request_to_pdu_bytes(&Request::WriteMultipleRegisters( + let bytes = encode_request_pdu_to_bytes(&Request::WriteMultipleRegisters( 0x06, Cow::Borrowed(&[0xABCD, 0xEF12]), )); @@ -685,14 +715,14 @@ mod tests { #[test] fn report_server_id() { - let bytes: Bytes = request_to_pdu_bytes(&Request::ReportServerId); + let bytes = encode_request_pdu_to_bytes(&Request::ReportServerId); assert_eq!(bytes[0], 0x11); } #[test] fn masked_write_register() { - let bytes: Bytes = - request_to_pdu_bytes(&Request::MaskWriteRegister(0xABCD, 0xEF12, 0x2345)); + let bytes = + encode_request_pdu_to_bytes(&Request::MaskWriteRegister(0xABCD, 0xEF12, 0x2345)); // function code assert_eq!(bytes[0], 0x16); @@ -713,7 +743,7 @@ mod tests { #[test] fn read_write_multiple_registers() { let data = [0xABCD, 0xEF12]; - let bytes: Bytes = request_to_pdu_bytes(&Request::ReadWriteMultipleRegisters( + let bytes = encode_request_pdu_to_bytes(&Request::ReadWriteMultipleRegisters( 0x05, 51, 0x03, @@ -751,7 +781,7 @@ mod tests { #[test] fn custom() { - let bytes: Bytes = request_to_pdu_bytes(&Request::Custom( + let bytes = encode_request_pdu_to_bytes(&Request::Custom( 0x55, Cow::Borrowed(&[0xCC, 0x88, 0xAA, 0xFF]), )); @@ -903,7 +933,9 @@ mod tests { #[test] fn read_coils() { - let bytes: Bytes = Response::ReadCoils(vec![true, false, false, true, false]).into(); + let bytes = encode_response_pdu_to_bytes(&Response::ReadCoils(vec![ + true, false, false, true, false, + ])); assert_eq!(bytes[0], 1); assert_eq!(bytes[1], 1); assert_eq!(bytes[2], 0b_0000_1001); @@ -911,7 +943,9 @@ mod tests { #[test] fn read_discrete_inputs() { - let bytes: Bytes = Response::ReadDiscreteInputs(vec![true, false, true, true]).into(); + let bytes = encode_response_pdu_to_bytes(&Response::ReadDiscreteInputs(vec![ + true, false, true, true, + ])); assert_eq!(bytes[0], 2); assert_eq!(bytes[1], 1); assert_eq!(bytes[2], 0b_0000_1101); @@ -919,7 +953,7 @@ mod tests { #[test] fn write_single_coil() { - let bytes: Bytes = Response::WriteSingleCoil(0x33, true).into(); + let bytes = encode_response_pdu_to_bytes(&Response::WriteSingleCoil(0x33, true)); assert_eq!(bytes[0], 5); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x33); @@ -929,7 +963,7 @@ mod tests { #[test] fn write_multiple_coils() { - let bytes: Bytes = Response::WriteMultipleCoils(0x3311, 5).into(); + let bytes = encode_response_pdu_to_bytes(&Response::WriteMultipleCoils(0x3311, 5)); assert_eq!(bytes[0], 0x0F); assert_eq!(bytes[1], 0x33); assert_eq!(bytes[2], 0x11); @@ -939,7 +973,9 @@ mod tests { #[test] fn read_input_registers() { - let bytes: Bytes = Response::ReadInputRegisters(vec![0xAA00, 0xCCBB, 0xEEDD]).into(); + let bytes = encode_response_pdu_to_bytes(&Response::ReadInputRegisters(vec![ + 0xAA00, 0xCCBB, 0xEEDD, + ])); assert_eq!(bytes[0], 4); assert_eq!(bytes[1], 0x06); assert_eq!(bytes[2], 0xAA); @@ -952,7 +988,8 @@ mod tests { #[test] fn read_holding_registers() { - let bytes: Bytes = Response::ReadHoldingRegisters(vec![0xAA00, 0x1111]).into(); + let bytes = + encode_response_pdu_to_bytes(&Response::ReadHoldingRegisters(vec![0xAA00, 0x1111])); assert_eq!(bytes[0], 3); assert_eq!(bytes[1], 0x04); assert_eq!(bytes[2], 0xAA); @@ -963,7 +1000,7 @@ mod tests { #[test] fn write_single_register() { - let bytes: Bytes = Response::WriteSingleRegister(0x07, 0xABCD).into(); + let bytes = encode_response_pdu_to_bytes(&Response::WriteSingleRegister(0x07, 0xABCD)); assert_eq!(bytes[0], 6); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x07); @@ -973,7 +1010,7 @@ mod tests { #[test] fn write_multiple_registers() { - let bytes: Bytes = Response::WriteMultipleRegisters(0x06, 2).into(); + let bytes = encode_response_pdu_to_bytes(&Response::WriteMultipleRegisters(0x06, 2)); assert_eq!(bytes[0], 0x10); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x06); @@ -983,7 +1020,11 @@ mod tests { #[test] fn report_server_id() { - let bytes: Bytes = Response::ReportServerId(0x42, true, vec![0x10, 0x20]).into(); + let bytes = encode_response_pdu_to_bytes(&Response::ReportServerId( + 0x42, + true, + vec![0x10, 0x20], + )); assert_eq!(bytes[0], 0x11); assert_eq!(bytes[1], 0x04); assert_eq!(bytes[2], 0x42); @@ -994,7 +1035,8 @@ mod tests { #[test] fn masked_write_register() { - let bytes: Bytes = Response::MaskWriteRegister(0x06, 0x8001, 0x4002).into(); + let bytes = + encode_response_pdu_to_bytes(&Response::MaskWriteRegister(0x06, 0x8001, 0x4002)); assert_eq!(bytes[0], 0x16); assert_eq!(bytes[1], 0x00); assert_eq!(bytes[2], 0x06); @@ -1006,7 +1048,8 @@ mod tests { #[test] fn read_write_multiple_registers() { - let bytes: Bytes = Response::ReadWriteMultipleRegisters(vec![0x1234]).into(); + let bytes = + encode_response_pdu_to_bytes(&Response::ReadWriteMultipleRegisters(vec![0x1234])); assert_eq!(bytes[0], 0x17); assert_eq!(bytes[1], 0x02); assert_eq!(bytes[2], 0x12); @@ -1015,8 +1058,10 @@ mod tests { #[test] fn custom() { - let bytes: Bytes = - Response::Custom(0x55, Bytes::from_static(&[0xCC, 0x88, 0xAA, 0xFF])).into(); + let bytes = encode_response_pdu_to_bytes(&Response::Custom( + 0x55, + Bytes::from_static(&[0xCC, 0x88, 0xAA, 0xFF]), + )); assert_eq!(bytes[0], 0x55); assert_eq!(bytes[1], 0xCC); assert_eq!(bytes[2], 0x88); diff --git a/src/codec/rtu.rs b/src/codec/rtu.rs index 7a5450c..fc766b6 100644 --- a/src/codec/rtu.rs +++ b/src/codec/rtu.rs @@ -3,7 +3,7 @@ use std::io::{Cursor, Error, ErrorKind, Result}; -use byteorder::BigEndian; +use byteorder::{BigEndian, ReadBytesExt as _}; use smallvec::SmallVec; use tokio_util::codec::{Decoder, Encoder}; @@ -13,7 +13,7 @@ use crate::{ slave::SlaveId, }; -use super::*; +use super::{encode_request_pdu, request_pdu_size, RequestPdu}; // [Modbus over Serial Line Specification and Implementation Guide V1.02](http://modbus.org/docs/Modbus_over_serial_line_V1_02.pdf), page 13 // "The maximum size of a Modbus RTU frame is 256 bytes." @@ -289,7 +289,7 @@ impl Decoder for ClientCodec { // Decoding of the PDU is unlikely to fail due // to transmission errors, because the frame's bytes // have already been verified with the CRC. - ResponsePdu::try_from(pdu_data) + super::ResponsePdu::try_from(pdu_data) .map(|pdu| Some(ResponseAdu { hdr, pdu })) .map_err(|err| { // Unrecoverable error @@ -314,7 +314,7 @@ impl Decoder for ServerCodec { // Decoding of the PDU is unlikely to fail due // to transmission errors, because the frame's bytes // have already been verified with the CRC. - RequestPdu::try_from(pdu_data) + super::RequestPdu::try_from(pdu_data) .map(|pdu| Some(RequestAdu { hdr, pdu })) .map_err(|err| { // Unrecoverable error @@ -348,12 +348,15 @@ impl Encoder for ServerCodec { type Error = Error; fn encode(&mut self, adu: ResponseAdu, buf: &mut BytesMut) -> Result<()> { - let ResponseAdu { hdr, pdu } = adu; - let pdu_data: Bytes = pdu.into(); - buf.reserve(pdu_data.len() + 3); + let ResponseAdu { + hdr, + pdu: super::ResponsePdu(pdu_res), + } = adu; + let buf_offset = buf.len(); + buf.reserve(super::response_result_pdu_size(&pdu_res) + 3); buf.put_u8(hdr.slave_id); - buf.put_slice(&pdu_data); - let crc = calc_crc(buf); + super::encode_response_result_pdu(buf, &pdu_res); + let crc = calc_crc(&buf[buf_offset..]); buf.put_u16(crc); Ok(()) } @@ -510,6 +513,8 @@ mod tests { mod client { + use crate::{codec::ResponsePdu, Request, Response}; + use super::*; #[test] diff --git a/src/codec/tcp.rs b/src/codec/tcp.rs index eb9a2ce..259f9b9 100644 --- a/src/codec/tcp.rs +++ b/src/codec/tcp.rs @@ -148,14 +148,17 @@ impl Encoder for ServerCodec { type Error = Error; fn encode(&mut self, adu: ResponseAdu, buf: &mut BytesMut) -> Result<()> { - let ResponseAdu { hdr, pdu } = adu; - let pdu_data: Bytes = pdu.into(); - buf.reserve(pdu_data.len() + 7); + let ResponseAdu { + hdr, + pdu: ResponsePdu(pdu_result), + } = adu; + let response_result_pdu_size = super::response_result_pdu_size(&pdu_result); + buf.reserve(response_result_pdu_size + 7); buf.put_u16(hdr.transaction_id); buf.put_u16(PROTOCOL_ID); - buf.put_u16(u16_len(pdu_data.len() + 1)); + buf.put_u16(u16_len(response_result_pdu_size + 1)); buf.put_u8(hdr.unit_id); - buf.put_slice(&pdu_data); + super::encode_response_result_pdu(buf, &pdu_result); Ok(()) } } From b9cb0116927471e0535de1f52b8fa1cf204e347e Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 14 Oct 2024 13:40:09 +0200 Subject: [PATCH 04/13] Rename function --- src/codec/mod.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 8549861..471c4f2 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -185,7 +185,7 @@ impl TryFrom for Request<'static> { return Err(Error::new(ErrorKind::InvalidData, "Invalid byte count")); } let x = &bytes[6..]; - WriteMultipleCoils(address, unpack_coils(x, quantity).into()) + WriteMultipleCoils(address, decode_packed_coils(x, quantity).into()) } 0x04 => ReadInputRegisters(rdr.read_u16::()?, rdr.read_u16::()?), 0x03 => { @@ -263,7 +263,7 @@ impl TryFrom for Response { // Here we have not information about the exact requested quantity so we just // unpack the whole byte. let quantity = u16::from(byte_count) * 8; - ReadCoils(unpack_coils(x, quantity)) + ReadCoils(decode_packed_coils(x, quantity)) } 0x02 => { let byte_count = rdr.read_u8()?; @@ -271,7 +271,7 @@ impl TryFrom for Response { // Here we have no information about the exact requested quantity so we just // unpack the whole byte. let quantity = u16::from(byte_count) * 8; - ReadDiscreteInputs(unpack_coils(x, quantity)) + ReadDiscreteInputs(decode_packed_coils(x, quantity)) } 0x05 => WriteSingleCoil( rdr.read_u16::()?, @@ -415,7 +415,7 @@ fn encode_packed_coils(buf: &mut crate::bytes::BytesMut, coils: &[Coil]) -> usiz packed_coils_size } -fn unpack_coils(bytes: &[u8], count: u16) -> Vec { +fn decode_packed_coils(bytes: &[u8], count: u16) -> Vec { let mut res = Vec::with_capacity(count.into()); for i in 0usize..count.into() { res.push((bytes[i / 8] >> (i % 8)) & 0b1 > 0); @@ -532,13 +532,13 @@ mod tests { #[test] fn test_unpack_bits() { - assert_eq!(unpack_coils(&[], 0), &[]); - assert_eq!(unpack_coils(&[0, 0], 0), &[]); - assert_eq!(unpack_coils(&[0b1], 1), &[true]); - assert_eq!(unpack_coils(&[0b01], 2), &[true, false]); - assert_eq!(unpack_coils(&[0b10], 2), &[false, true]); - assert_eq!(unpack_coils(&[0b101], 3), &[true, false, true]); - assert_eq!(unpack_coils(&[0xff, 0b11], 10), &[true; 10]); + assert_eq!(decode_packed_coils(&[], 0), &[]); + assert_eq!(decode_packed_coils(&[0, 0], 0), &[]); + assert_eq!(decode_packed_coils(&[0b1], 1), &[true]); + assert_eq!(decode_packed_coils(&[0b01], 2), &[true, false]); + assert_eq!(decode_packed_coils(&[0b10], 2), &[false, true]); + assert_eq!(decode_packed_coils(&[0b101], 3), &[true, false, true]); + assert_eq!(decode_packed_coils(&[0xff, 0b11], 10), &[true; 10]); } #[test] From fa1089c8894141a5174c0804b250c7477cf3f597 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 14 Oct 2024 13:50:48 +0200 Subject: [PATCH 05/13] Extract function to improve readability --- CHANGELOG.md | 3 + src/codec/mod.rs | 148 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 103 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 806734a..3fab632 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ ## v0.16.0 (Unreleased) +- Request encoding: Reduced allocations +- Response decoding: More strict validation + ### Breaking Changes - Increased MSRV from 1.65 to 1.76 diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 471c4f2..3640a1c 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -9,7 +9,7 @@ use std::{ use byteorder::{BigEndian, ReadBytesExt as _}; use crate::{ - bytes::Bytes, + bytes::{Buf as _, Bytes}, frame::{Coil, RequestPdu, ResponsePdu}, ExceptionCode, ExceptionResponse, FunctionCode, Request, Response, }; @@ -163,23 +163,24 @@ fn encode_response_result_pdu( } } +fn read_u16_be(reader: &mut impl io::Read) -> io::Result { + reader.read_u16::() +} + impl TryFrom for Request<'static> { type Error = Error; fn try_from(bytes: Bytes) -> Result { use crate::frame::Request::*; - let mut rdr = Cursor::new(&bytes); + let rdr = &mut Cursor::new(&bytes); let fn_code = rdr.read_u8()?; let req = match fn_code { - 0x01 => ReadCoils(rdr.read_u16::()?, rdr.read_u16::()?), - 0x02 => ReadDiscreteInputs(rdr.read_u16::()?, rdr.read_u16::()?), - 0x05 => WriteSingleCoil( - rdr.read_u16::()?, - coil_to_bool(rdr.read_u16::()?)?, - ), + 0x01 => ReadCoils(read_u16_be(rdr)?, read_u16_be(rdr)?), + 0x02 => ReadDiscreteInputs(read_u16_be(rdr)?, read_u16_be(rdr)?), + 0x05 => WriteSingleCoil(read_u16_be(rdr)?, coil_to_bool(read_u16_be(rdr)?)?), 0x0F => { - let address = rdr.read_u16::()?; - let quantity = rdr.read_u16::()?; + let address = read_u16_be(rdr)?; + let quantity = read_u16_be(rdr)?; let byte_count = rdr.read_u8()?; if bytes.len() < 6 + usize::from(byte_count) { return Err(Error::new(ErrorKind::InvalidData, "Invalid byte count")); @@ -187,44 +188,42 @@ impl TryFrom for Request<'static> { let x = &bytes[6..]; WriteMultipleCoils(address, decode_packed_coils(x, quantity).into()) } - 0x04 => ReadInputRegisters(rdr.read_u16::()?, rdr.read_u16::()?), - 0x03 => { - ReadHoldingRegisters(rdr.read_u16::()?, rdr.read_u16::()?) - } - 0x06 => WriteSingleRegister(rdr.read_u16::()?, rdr.read_u16::()?), + 0x04 => ReadInputRegisters(read_u16_be(rdr)?, read_u16_be(rdr)?), + 0x03 => ReadHoldingRegisters(read_u16_be(rdr)?, read_u16_be(rdr)?), + 0x06 => WriteSingleRegister(read_u16_be(rdr)?, read_u16_be(rdr)?), 0x10 => { - let address = rdr.read_u16::()?; - let quantity = rdr.read_u16::()?; + let address = read_u16_be(rdr)?; + let quantity = read_u16_be(rdr)?; let byte_count = rdr.read_u8()?; if bytes.len() < 6 + usize::from(byte_count) { return Err(Error::new(ErrorKind::InvalidData, "Invalid byte count")); } let mut data = Vec::with_capacity(quantity.into()); for _ in 0..quantity { - data.push(rdr.read_u16::()?); + data.push(read_u16_be(rdr)?); } WriteMultipleRegisters(address, data.into()) } 0x11 => ReportServerId, 0x16 => { - let address = rdr.read_u16::()?; - let and_mask = rdr.read_u16::()?; - let or_mask = rdr.read_u16::()?; + let address = read_u16_be(rdr)?; + let and_mask = read_u16_be(rdr)?; + let or_mask = read_u16_be(rdr)?; MaskWriteRegister(address, and_mask, or_mask) } 0x17 => { - let read_address = rdr.read_u16::()?; - let read_quantity = rdr.read_u16::()?; - let write_address = rdr.read_u16::()?; - let write_quantity = rdr.read_u16::()?; + let read_address = read_u16_be(rdr)?; + let read_quantity = read_u16_be(rdr)?; + let write_address = read_u16_be(rdr)?; + let write_quantity = read_u16_be(rdr)?; let write_count = rdr.read_u8()?; if bytes.len() < 10 + usize::from(write_count) { return Err(Error::new(ErrorKind::InvalidData, "Invalid byte count")); } let mut data = Vec::with_capacity(write_quantity.into()); for _ in 0..write_quantity { - data.push(rdr.read_u16::()?); + data.push(read_u16_be(rdr)?); } ReadWriteMultipleRegisters(read_address, read_quantity, write_address, data.into()) } @@ -252,57 +251,93 @@ impl TryFrom for RequestPdu<'static> { impl TryFrom for Response { type Error = Error; + #[allow(clippy::too_many_lines)] // TODO fn try_from(bytes: Bytes) -> Result { use crate::frame::Response::*; - let mut rdr = Cursor::new(&bytes); + let rdr = &mut Cursor::new(&bytes); let fn_code = rdr.read_u8()?; let rsp = match fn_code { 0x01 => { let byte_count = rdr.read_u8()?; - let x = &bytes[2..]; + let packed_coils = &bytes[2..]; + if packed_coils.len() != byte_count.into() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); + } // Here we have not information about the exact requested quantity so we just // unpack the whole byte. let quantity = u16::from(byte_count) * 8; - ReadCoils(decode_packed_coils(x, quantity)) + ReadCoils(decode_packed_coils(packed_coils, quantity)) } 0x02 => { let byte_count = rdr.read_u8()?; - let x = &bytes[2..]; + let packed_coils = &bytes[2..]; + if packed_coils.len() != byte_count.into() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); + } // Here we have no information about the exact requested quantity so we just // unpack the whole byte. let quantity = u16::from(byte_count) * 8; - ReadDiscreteInputs(decode_packed_coils(x, quantity)) + ReadDiscreteInputs(decode_packed_coils(packed_coils, quantity)) } - 0x05 => WriteSingleCoil( - rdr.read_u16::()?, - coil_to_bool(rdr.read_u16::()?)?, - ), - 0x0F => WriteMultipleCoils(rdr.read_u16::()?, rdr.read_u16::()?), + 0x05 => WriteSingleCoil(read_u16_be(rdr)?, coil_to_bool(read_u16_be(rdr)?)?), + 0x0F => WriteMultipleCoils(read_u16_be(rdr)?, read_u16_be(rdr)?), 0x04 => { let byte_count = rdr.read_u8()?; + if byte_count % 2 != 0 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); + } let quantity = byte_count / 2; let mut data = Vec::with_capacity(quantity.into()); for _ in 0..quantity { - data.push(rdr.read_u16::()?); + data.push(read_u16_be(rdr)?); + } + if rdr.has_remaining() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); } ReadInputRegisters(data) } 0x03 => { let byte_count = rdr.read_u8()?; + if byte_count % 2 != 0 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); + } let quantity = byte_count / 2; let mut data = Vec::with_capacity(quantity.into()); for _ in 0..quantity { - data.push(rdr.read_u16::()?); + data.push(read_u16_be(rdr)?); + } + if rdr.has_remaining() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); } ReadHoldingRegisters(data) } - 0x06 => WriteSingleRegister(rdr.read_u16::()?, rdr.read_u16::()?), + 0x06 => WriteSingleRegister(read_u16_be(rdr)?, read_u16_be(rdr)?), - 0x10 => { - WriteMultipleRegisters(rdr.read_u16::()?, rdr.read_u16::()?) - } + 0x10 => WriteMultipleRegisters(read_u16_be(rdr)?, read_u16_be(rdr)?), 0x11 => { let byte_count = rdr.read_u8()?; + if byte_count < 2 { + return Err(io::Error::new(io::ErrorKind::InvalidData, "too short")); + } + let data_len = (byte_count - 2).into(); let server_id = rdr.read_u8()?; let run_indication_status = match rdr.read_u8()? { 0x00 => false, @@ -310,29 +345,46 @@ impl TryFrom for Response { status => { return Err(Error::new( ErrorKind::InvalidData, - format!("Invalid run indication status value: {status}"), + format!("invalid run indication status: 0x{status:02X}"), )); } }; - let data_len = byte_count.saturating_sub(2).into(); let mut data = Vec::with_capacity(data_len); for _ in 0..data_len { data.push(rdr.read_u8()?); } + if rdr.has_remaining() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); + } ReportServerId(server_id, run_indication_status, data) } 0x16 => { - let address = rdr.read_u16::()?; - let and_mask = rdr.read_u16::()?; - let or_mask = rdr.read_u16::()?; + let address = read_u16_be(rdr)?; + let and_mask = read_u16_be(rdr)?; + let or_mask = read_u16_be(rdr)?; MaskWriteRegister(address, and_mask, or_mask) } 0x17 => { let byte_count = rdr.read_u8()?; + if byte_count % 2 != 0 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); + } let quantity = byte_count / 2; let mut data = Vec::with_capacity(quantity.into()); for _ in 0..quantity { - data.push(rdr.read_u16::()?); + data.push(read_u16_be(rdr)?); + } + if rdr.has_remaining() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid quantity", + )); } ReadWriteMultipleRegisters(data) } From d7ac8e2bad008b061b336e2b07fbefa499dd955a Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 15 Oct 2024 11:47:26 +0200 Subject: [PATCH 06/13] Verify exact frame length on decoding --- CHANGELOG.md | 2 +- src/codec/mod.rs | 52 ++++++++++++++++++++---------------------------- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fab632..10c6a6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ## v0.16.0 (Unreleased) - Request encoding: Reduced allocations -- Response decoding: More strict validation +- Request/response decoding: More strict validation ### Breaking Changes diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 3640a1c..b5ae00a 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -183,7 +183,7 @@ impl TryFrom for Request<'static> { let quantity = read_u16_be(rdr)?; let byte_count = rdr.read_u8()?; if bytes.len() < 6 + usize::from(byte_count) { - return Err(Error::new(ErrorKind::InvalidData, "Invalid byte count")); + return Err(Error::new(ErrorKind::InvalidData, "too short")); } let x = &bytes[6..]; WriteMultipleCoils(address, decode_packed_coils(x, quantity).into()) @@ -197,7 +197,7 @@ impl TryFrom for Request<'static> { let quantity = read_u16_be(rdr)?; let byte_count = rdr.read_u8()?; if bytes.len() < 6 + usize::from(byte_count) { - return Err(Error::new(ErrorKind::InvalidData, "Invalid byte count")); + return Err(Error::new(ErrorKind::InvalidData, "too short")); } let mut data = Vec::with_capacity(quantity.into()); for _ in 0..quantity { @@ -219,7 +219,7 @@ impl TryFrom for Request<'static> { let write_quantity = read_u16_be(rdr)?; let write_count = rdr.read_u8()?; if bytes.len() < 10 + usize::from(write_count) { - return Err(Error::new(ErrorKind::InvalidData, "Invalid byte count")); + return Err(Error::new(ErrorKind::InvalidData, "too short")); } let mut data = Vec::with_capacity(write_quantity.into()); for _ in 0..write_quantity { @@ -227,14 +227,23 @@ impl TryFrom for Request<'static> { } ReadWriteMultipleRegisters(read_address, read_quantity, write_address, data.into()) } - fn_code if fn_code < 0x80 => Custom(fn_code, bytes[1..].to_vec().into()), + fn_code if fn_code < 0x80 => { + return Ok(Custom(fn_code, bytes[1..].to_vec().into())); + } fn_code => { return Err(Error::new( ErrorKind::InvalidData, - format!("Invalid function code: 0x{fn_code:0>2X}"), + format!("invalid function code: 0x{fn_code:02X}"), )); } }; + // Verify that all data has been consumed and decoded. + if rdr.has_remaining() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "undecoded request data", + )); + } Ok(req) } } @@ -300,12 +309,6 @@ impl TryFrom for Response { for _ in 0..quantity { data.push(read_u16_be(rdr)?); } - if rdr.has_remaining() { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "invalid quantity", - )); - } ReadInputRegisters(data) } 0x03 => { @@ -321,12 +324,6 @@ impl TryFrom for Response { for _ in 0..quantity { data.push(read_u16_be(rdr)?); } - if rdr.has_remaining() { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "invalid quantity", - )); - } ReadHoldingRegisters(data) } 0x06 => WriteSingleRegister(read_u16_be(rdr)?, read_u16_be(rdr)?), @@ -353,12 +350,6 @@ impl TryFrom for Response { for _ in 0..data_len { data.push(rdr.read_u8()?); } - if rdr.has_remaining() { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "invalid quantity", - )); - } ReportServerId(server_id, run_indication_status, data) } 0x16 => { @@ -380,19 +371,20 @@ impl TryFrom for Response { for _ in 0..quantity { data.push(read_u16_be(rdr)?); } - if rdr.has_remaining() { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "invalid quantity", - )); - } ReadWriteMultipleRegisters(data) } _ => { let mut bytes = bytes; - Custom(fn_code, bytes.split_off(1)) + return Ok(Custom(fn_code, bytes.split_off(1))); } }; + // Verify that all data has been consumed and decoded. + if rdr.has_remaining() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "undecoded response data", + )); + } Ok(rsp) } } From acc27d63abc39e815b01091ecc162706a2cf7ede Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 15 Oct 2024 12:25:32 +0200 Subject: [PATCH 07/13] Limit PDU size to 253 bytes --- CHANGELOG.md | 5 ++- src/codec/mod.rs | 95 ++++++++++++++++++++++++++++++++---------------- src/codec/rtu.rs | 5 ++- src/codec/tcp.rs | 4 +- 4 files changed, 72 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10c6a6e..0385df7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,9 @@ ## v0.16.0 (Unreleased) -- Request encoding: Reduced allocations -- Request/response decoding: More strict validation +- Encoding of requests: Less allocations +- Decoding of requests/responses: More strict validation +- Limit request/response PDU size to 253 bytes (encoding/decoding) ### Breaking Changes diff --git a/src/codec/mod.rs b/src/codec/mod.rs index b5ae00a..3b5f6df 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -3,7 +3,7 @@ use std::{ convert::TryFrom, - io::{self, Cursor, Error, ErrorKind}, + io::{self, BufRead as _, Cursor, Error, ErrorKind}, }; use byteorder::{BigEndian, ReadBytesExt as _}; @@ -20,6 +20,11 @@ pub(crate) mod rtu; #[cfg(feature = "tcp")] pub(crate) mod tcp; +/// Maximum request/response PDU size. +/// +/// As defined by the spec for both RTU and TCP. +const MAX_PDU_SIZE: usize = 253; + #[cfg(any(test, feature = "rtu", feature = "tcp"))] #[allow(clippy::cast_possible_truncation)] fn u16_len(len: usize) -> u16 { @@ -85,9 +90,9 @@ fn encode_request_pdu(buf: &mut crate::bytes::BytesMut, request: &Request<'_>) { buf.put_u16(*read_address); buf.put_u16(*quantity); buf.put_u16(*write_address); - let n = words.len(); - buf.put_u16(u16_len(n)); - buf.put_u8(u8_len(n * 2)); + let len = words.len(); + buf.put_u16(u16_len(len)); + buf.put_u8(u8_len(len * 2)); for w in words.as_ref() { buf.put_u16(*w); } @@ -172,6 +177,12 @@ impl TryFrom for Request<'static> { fn try_from(bytes: Bytes) -> Result { use crate::frame::Request::*; + if bytes.len() > MAX_PDU_SIZE { + return Err(io::Error::new( + ErrorKind::InvalidData, + "request PDU size exceeded", + )); + } let rdr = &mut Cursor::new(&bytes); let fn_code = rdr.read_u8()?; let req = match fn_code { @@ -181,12 +192,13 @@ impl TryFrom for Request<'static> { 0x0F => { let address = read_u16_be(rdr)?; let quantity = read_u16_be(rdr)?; - let byte_count = rdr.read_u8()?; - if bytes.len() < 6 + usize::from(byte_count) { - return Err(Error::new(ErrorKind::InvalidData, "too short")); + let byte_count = usize::from(rdr.read_u8()?); + if bytes.len() < 6 + byte_count { + return Err(io::Error::new(ErrorKind::InvalidData, "too short")); } - let x = &bytes[6..]; - WriteMultipleCoils(address, decode_packed_coils(x, quantity).into()) + rdr.consume(byte_count); + let packed_coils = &bytes[6..6 + byte_count]; + WriteMultipleCoils(address, decode_packed_coils(packed_coils, quantity).into()) } 0x04 => ReadInputRegisters(read_u16_be(rdr)?, read_u16_be(rdr)?), 0x03 => ReadHoldingRegisters(read_u16_be(rdr)?, read_u16_be(rdr)?), @@ -196,8 +208,8 @@ impl TryFrom for Request<'static> { let address = read_u16_be(rdr)?; let quantity = read_u16_be(rdr)?; let byte_count = rdr.read_u8()?; - if bytes.len() < 6 + usize::from(byte_count) { - return Err(Error::new(ErrorKind::InvalidData, "too short")); + if u16::from(byte_count) != quantity * 2 { + return Err(io::Error::new(ErrorKind::InvalidData, "invalid quantity")); } let mut data = Vec::with_capacity(quantity.into()); for _ in 0..quantity { @@ -218,8 +230,11 @@ impl TryFrom for Request<'static> { let write_address = read_u16_be(rdr)?; let write_quantity = read_u16_be(rdr)?; let write_count = rdr.read_u8()?; - if bytes.len() < 10 + usize::from(write_count) { - return Err(Error::new(ErrorKind::InvalidData, "too short")); + if u16::from(write_count) != write_quantity * 2 { + return Err(io::Error::new( + ErrorKind::InvalidData, + "invalid write quantity", + )); } let mut data = Vec::with_capacity(write_quantity.into()); for _ in 0..write_quantity { @@ -228,6 +243,7 @@ impl TryFrom for Request<'static> { ReadWriteMultipleRegisters(read_address, read_quantity, write_address, data.into()) } fn_code if fn_code < 0x80 => { + // Consume all remaining bytes as custom data. return Ok(Custom(fn_code, bytes[1..].to_vec().into())); } fn_code => { @@ -263,18 +279,22 @@ impl TryFrom for Response { #[allow(clippy::too_many_lines)] // TODO fn try_from(bytes: Bytes) -> Result { use crate::frame::Response::*; + if bytes.len() > MAX_PDU_SIZE { + return Err(io::Error::new( + ErrorKind::InvalidInput, + "response PDU size exceeded", + )); + } let rdr = &mut Cursor::new(&bytes); let fn_code = rdr.read_u8()?; let rsp = match fn_code { 0x01 => { let byte_count = rdr.read_u8()?; - let packed_coils = &bytes[2..]; - if packed_coils.len() != byte_count.into() { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "invalid quantity", - )); + if bytes.len() < 2 + usize::from(byte_count) { + return Err(io::Error::new(ErrorKind::InvalidData, "too short")); } + let packed_coils = &bytes[2..2 + usize::from(byte_count)]; + rdr.consume(byte_count.into()); // Here we have not information about the exact requested quantity so we just // unpack the whole byte. let quantity = u16::from(byte_count) * 8; @@ -282,13 +302,11 @@ impl TryFrom for Response { } 0x02 => { let byte_count = rdr.read_u8()?; - let packed_coils = &bytes[2..]; - if packed_coils.len() != byte_count.into() { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "invalid quantity", - )); + if bytes.len() < 2 + usize::from(byte_count) { + return Err(io::Error::new(ErrorKind::InvalidData, "too short")); } + let packed_coils = &bytes[2..2 + usize::from(byte_count)]; + rdr.consume(byte_count.into()); // Here we have no information about the exact requested quantity so we just // unpack the whole byte. let quantity = u16::from(byte_count) * 8; @@ -374,6 +392,7 @@ impl TryFrom for Response { ReadWriteMultipleRegisters(data) } _ => { + // Consume all remaining bytes as custom data. let mut bytes = bytes; return Ok(Custom(fn_code, bytes.split_off(1))); } @@ -468,9 +487,9 @@ fn decode_packed_coils(bytes: &[u8], count: u16) -> Vec { } #[cfg(any(feature = "rtu", feature = "tcp"))] -fn request_pdu_size(req: &Request<'_>) -> usize { +fn request_pdu_size(req: &Request<'_>) -> io::Result { use crate::frame::Request::*; - match req { + let size = match req { ReadCoils(_, _) | ReadDiscreteInputs(_, _) | ReadInputRegisters(_, _) @@ -483,13 +502,20 @@ fn request_pdu_size(req: &Request<'_>) -> usize { MaskWriteRegister(_, _, _) => 7, ReadWriteMultipleRegisters(_, _, _, data) => 10 + data.len() * 2, Custom(_, data) => 1 + data.len(), + }; + if size > MAX_PDU_SIZE { + return Err(io::Error::new( + ErrorKind::InvalidInput, + "request PDU size exceeded", + )); } + Ok(size) } #[cfg(feature = "server")] -fn response_pdu_size(rsp: &Response) -> usize { +fn response_pdu_size(rsp: &Response) -> io::Result { use crate::frame::Response::*; - match rsp { + let size = match rsp { ReadCoils(coils) | ReadDiscreteInputs(coils) => 2 + packed_coils_size(coils), WriteSingleCoil(_, _) | WriteMultipleCoils(_, _) @@ -501,14 +527,21 @@ fn response_pdu_size(rsp: &Response) -> usize { ReportServerId(_, _, ref data) => 3 + data.len(), MaskWriteRegister(_, _, _) => 7, Custom(_, ref data) => 1 + data.len(), + }; + if size > MAX_PDU_SIZE { + return Err(io::Error::new( + ErrorKind::InvalidInput, + "response PDU size exceeded", + )); } + Ok(size) } #[cfg(feature = "server")] -fn response_result_pdu_size(res: &Result) -> usize { +fn response_result_pdu_size(res: &Result) -> io::Result { match res { Ok(rsp) => response_pdu_size(rsp), - Err(_) => 2, + Err(_) => Ok(2), } } diff --git a/src/codec/rtu.rs b/src/codec/rtu.rs index fc766b6..67a2af2 100644 --- a/src/codec/rtu.rs +++ b/src/codec/rtu.rs @@ -333,7 +333,7 @@ impl<'a> Encoder> for ClientCodec { pdu: RequestPdu(request), } = adu; let buf_offset = buf.len(); - let request_pdu_size = request_pdu_size(&request); + let request_pdu_size = request_pdu_size(&request)?; buf.reserve((buf.capacity() - buf_offset) + request_pdu_size + 3); buf.put_u8(hdr.slave_id); encode_request_pdu(buf, &request); @@ -353,7 +353,8 @@ impl Encoder for ServerCodec { pdu: super::ResponsePdu(pdu_res), } = adu; let buf_offset = buf.len(); - buf.reserve(super::response_result_pdu_size(&pdu_res) + 3); + let response_result_pdu_size = super::response_result_pdu_size(&pdu_res)?; + buf.reserve(response_result_pdu_size + 3); buf.put_u8(hdr.slave_id); super::encode_response_result_pdu(buf, &pdu_res); let crc = calc_crc(&buf[buf_offset..]); diff --git a/src/codec/tcp.rs b/src/codec/tcp.rs index 259f9b9..e0ccb57 100644 --- a/src/codec/tcp.rs +++ b/src/codec/tcp.rs @@ -132,7 +132,7 @@ impl<'a> Encoder> for ClientCodec { pdu: RequestPdu(request), } = adu; let buf_offset = buf.len(); - let request_pdu_size = request_pdu_size(&request); + let request_pdu_size = request_pdu_size(&request)?; buf.reserve((buf.capacity() - buf_offset) + request_pdu_size + 7); buf.put_u16(hdr.transaction_id); buf.put_u16(PROTOCOL_ID); @@ -152,7 +152,7 @@ impl Encoder for ServerCodec { hdr, pdu: ResponsePdu(pdu_result), } = adu; - let response_result_pdu_size = super::response_result_pdu_size(&pdu_result); + let response_result_pdu_size = super::response_result_pdu_size(&pdu_result)?; buf.reserve(response_result_pdu_size + 7); buf.put_u16(hdr.transaction_id); buf.put_u16(PROTOCOL_ID); From a03b45fc433dcc264f2c1e44d66d3d6ccedb3f1b Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 17 Oct 2024 14:39:52 +0200 Subject: [PATCH 08/13] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0385df7..fb88dee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,11 @@ - Encoding of requests: Less allocations - Decoding of requests/responses: More strict validation -- Limit request/response PDU size to 253 bytes (encoding/decoding) ### Breaking Changes - Increased MSRV from 1.65 to 1.76 +- Limit request/response PDU size to 253 bytes (encoding/decoding) ## v0.15.0 (2024-10-10) From 628fa68e220b82d6aec3af2e0ca3e80d488baa5d Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 17 Oct 2024 17:38:18 +0200 Subject: [PATCH 09/13] Update src/codec/mod.rs Co-authored-by: Markus Kohlhase --- src/codec/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 3b5f6df..e79e2c3 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -513,7 +513,7 @@ fn request_pdu_size(req: &Request<'_>) -> io::Result { } #[cfg(feature = "server")] -fn response_pdu_size(rsp: &Response) -> io::Result { +fn response_pdu_size(response: &Response) -> io::Result { use crate::frame::Response::*; let size = match rsp { ReadCoils(coils) | ReadDiscreteInputs(coils) => 2 + packed_coils_size(coils), From c9cb4e71e07cd18c085a847dfa4436fec17e50a6 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 17 Oct 2024 17:38:26 +0200 Subject: [PATCH 10/13] Update src/codec/mod.rs Co-authored-by: Markus Kohlhase --- src/codec/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index e79e2c3..322b2e8 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -104,7 +104,7 @@ fn encode_request_pdu(buf: &mut crate::bytes::BytesMut, request: &Request<'_>) { } #[cfg(any(test, feature = "server"))] -fn encode_response_pdu(buf: &mut crate::bytes::BytesMut, rsp: &Response) { +fn encode_response_pdu(buf: &mut crate::bytes::BytesMut, response: &Response) { use crate::{bytes::BufMut as _, frame::Response::*}; buf.put_u8(rsp.function_code().value()); match rsp { From 11fc29f885b6f954e96536c5fc0325cb3798f051 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 17 Oct 2024 17:38:32 +0200 Subject: [PATCH 11/13] Update src/codec/mod.rs Co-authored-by: Markus Kohlhase --- src/codec/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 322b2e8..337893b 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -150,7 +150,7 @@ fn encode_response_pdu(buf: &mut crate::bytes::BytesMut, response: &Response) { } #[cfg(any(test, feature = "server"))] -fn encode_exception_response_pdu(buf: &mut crate::bytes::BytesMut, rsp: ExceptionResponse) { +fn encode_exception_response_pdu(buf: &mut crate::bytes::BytesMut, response: ExceptionResponse) { use crate::bytes::BufMut as _; debug_assert!(rsp.function.value() < 0x80); buf.put_u8(rsp.function.value() + 0x80); From 48184350d94786afaaaff65a7ebe834f5ed78502 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 17 Oct 2024 17:38:39 +0200 Subject: [PATCH 12/13] Update src/codec/mod.rs Co-authored-by: Markus Kohlhase --- src/codec/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 337893b..7338a1c 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -487,7 +487,7 @@ fn decode_packed_coils(bytes: &[u8], count: u16) -> Vec { } #[cfg(any(feature = "rtu", feature = "tcp"))] -fn request_pdu_size(req: &Request<'_>) -> io::Result { +fn request_pdu_size(request: &Request<'_>) -> io::Result { use crate::frame::Request::*; let size = match req { ReadCoils(_, _) From 5adf1e16d028bea75b564aa9747e8546241e9646 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 17 Oct 2024 17:41:01 +0200 Subject: [PATCH 13/13] Renaming --- src/codec/mod.rs | 99 ++++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 46 deletions(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 7338a1c..ee97bb5 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -106,8 +106,8 @@ fn encode_request_pdu(buf: &mut crate::bytes::BytesMut, request: &Request<'_>) { #[cfg(any(test, feature = "server"))] fn encode_response_pdu(buf: &mut crate::bytes::BytesMut, response: &Response) { use crate::{bytes::BufMut as _, frame::Response::*}; - buf.put_u8(rsp.function_code().value()); - match rsp { + buf.put_u8(response.function_code().value()); + match response { ReadCoils(coils) | ReadDiscreteInputs(coils) => { buf.put_u8(u8_len(packed_coils_size(coils))); encode_packed_coils(buf, coils); @@ -152,9 +152,9 @@ fn encode_response_pdu(buf: &mut crate::bytes::BytesMut, response: &Response) { #[cfg(any(test, feature = "server"))] fn encode_exception_response_pdu(buf: &mut crate::bytes::BytesMut, response: ExceptionResponse) { use crate::bytes::BufMut as _; - debug_assert!(rsp.function.value() < 0x80); - buf.put_u8(rsp.function.value() + 0x80); - buf.put_u8(rsp.exception.into()); + debug_assert!(response.function.value() < 0x80); + buf.put_u8(response.function.value() + 0x80); + buf.put_u8(response.exception.into()); } #[cfg(feature = "server")] @@ -163,8 +163,8 @@ fn encode_response_result_pdu( res: &Result, ) { match res { - Ok(rsp) => encode_response_pdu(buf, rsp), - Err(rsp) => encode_exception_response_pdu(buf, *rsp), + Ok(response) => encode_response_pdu(buf, response), + Err(response) => encode_exception_response_pdu(buf, *response), } } @@ -287,7 +287,7 @@ impl TryFrom for Response { } let rdr = &mut Cursor::new(&bytes); let fn_code = rdr.read_u8()?; - let rsp = match fn_code { + let response = match fn_code { 0x01 => { let byte_count = rdr.read_u8()?; if bytes.len() < 2 + usize::from(byte_count) { @@ -404,7 +404,7 @@ impl TryFrom for Response { "undecoded response data", )); } - Ok(rsp) + Ok(response) } } @@ -489,7 +489,7 @@ fn decode_packed_coils(bytes: &[u8], count: u16) -> Vec { #[cfg(any(feature = "rtu", feature = "tcp"))] fn request_pdu_size(request: &Request<'_>) -> io::Result { use crate::frame::Request::*; - let size = match req { + let size = match request { ReadCoils(_, _) | ReadDiscreteInputs(_, _) | ReadInputRegisters(_, _) @@ -515,7 +515,7 @@ fn request_pdu_size(request: &Request<'_>) -> io::Result { #[cfg(feature = "server")] fn response_pdu_size(response: &Response) -> io::Result { use crate::frame::Response::*; - let size = match rsp { + let size = match response { ReadCoils(coils) | ReadDiscreteInputs(coils) => 2 + packed_coils_size(coils), WriteSingleCoil(_, _) | WriteMultipleCoils(_, _) @@ -540,7 +540,7 @@ fn response_pdu_size(response: &Response) -> io::Result { #[cfg(feature = "server")] fn response_result_pdu_size(res: &Result) -> io::Result { match res { - Ok(rsp) => response_pdu_size(rsp), + Ok(response) => response_pdu_size(response), Err(_) => Ok(2), } } @@ -633,9 +633,9 @@ mod tests { assert!(ExceptionResponse::try_from(Bytes::from(vec![0x79, 0x02])).is_err()); let bytes = Bytes::from(vec![0x83, 0x02]); - let rsp = ExceptionResponse::try_from(bytes).unwrap(); + let response = ExceptionResponse::try_from(bytes).unwrap(); assert_eq!( - rsp, + response, ExceptionResponse { function: FunctionCode::ReadHoldingRegisters, exception: ExceptionCode::IllegalDataAddress, @@ -646,7 +646,7 @@ mod tests { #[test] fn pdu_into_bytes() { let req_pdu = encode_request_pdu_to_bytes(&Request::ReadCoils(0x01, 5)); - let rsp_pdu = encode_response_pdu_to_bytes(&Response::ReadCoils(vec![])); + let response_pdu = encode_response_pdu_to_bytes(&Response::ReadCoils(vec![])); let ex_pdu = encode_exception_response_pdu_to_bytes(ExceptionResponse { function: FunctionCode::ReadHoldingRegisters, exception: ExceptionCode::ServerDeviceFailure, @@ -658,8 +658,8 @@ mod tests { assert_eq!(req_pdu[3], 0x00); assert_eq!(req_pdu[4], 0x05); - assert_eq!(rsp_pdu[0], 0x01); - assert_eq!(rsp_pdu[1], 0x00); + assert_eq!(response_pdu[0], 0x01); + assert_eq!(response_pdu[1], 0x00); assert_eq!(ex_pdu[0], 0x83); assert_eq!(ex_pdu[1], 0x04); @@ -679,7 +679,8 @@ mod tests { 0x01, Cow::Borrowed(&[0; 80]), )); - let _rsp_pdu = encode_response_pdu_to_bytes(&Response::ReadInputRegisters(vec![0; 80])); + let _response_pdu = + encode_response_pdu_to_bytes(&Response::ReadInputRegisters(vec![0; 80])); } mod serialize_requests { @@ -1154,9 +1155,9 @@ mod tests { #[test] fn read_coils() { let bytes = Bytes::from(vec![1, 1, 0b_0000_1001]); - let rsp = Response::try_from(bytes).unwrap(); + let response = Response::try_from(bytes).unwrap(); assert_eq!( - rsp, + response, Response::ReadCoils(vec![true, false, false, true, false, false, false, false]) ); } @@ -1169,16 +1170,16 @@ mod tests { let mut values: Vec = (0..byte_count).map(|_| 0b_1111_1111).collect(); raw.append(&mut values); let bytes = Bytes::from(raw); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::ReadCoils(vec![true; quantity])); + let response = Response::try_from(bytes).unwrap(); + assert_eq!(response, Response::ReadCoils(vec![true; quantity])); } #[test] fn read_discrete_inputs() { let bytes = Bytes::from(vec![2, 1, 0b_0000_1001]); - let rsp = Response::try_from(bytes).unwrap(); + let response = Response::try_from(bytes).unwrap(); assert_eq!( - rsp, + response, Response::ReadDiscreteInputs(vec![ true, false, false, true, false, false, false, false, ],) @@ -1193,30 +1194,30 @@ mod tests { let mut values: Vec = (0..byte_count).map(|_| 0b_1111_1111).collect(); raw.append(&mut values); let bytes = Bytes::from(raw); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::ReadDiscreteInputs(vec![true; quantity])); + let response = Response::try_from(bytes).unwrap(); + assert_eq!(response, Response::ReadDiscreteInputs(vec![true; quantity])); } #[test] fn write_single_coil() { let bytes = Bytes::from(vec![5, 0x00, 0x33, 0xFF, 0x00]); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::WriteSingleCoil(0x33, true)); + let response = Response::try_from(bytes).unwrap(); + assert_eq!(response, Response::WriteSingleCoil(0x33, true)); } #[test] fn write_multiple_coils() { let bytes = Bytes::from(vec![0x0F, 0x33, 0x11, 0x00, 0x05]); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::WriteMultipleCoils(0x3311, 5)); + let response = Response::try_from(bytes).unwrap(); + assert_eq!(response, Response::WriteMultipleCoils(0x3311, 5)); } #[test] fn read_input_registers() { let bytes = Bytes::from(vec![4, 0x06, 0xAA, 0x00, 0xCC, 0xBB, 0xEE, 0xDD]); - let rsp = Response::try_from(bytes).unwrap(); + let response = Response::try_from(bytes).unwrap(); assert_eq!( - rsp, + response, Response::ReadInputRegisters(vec![0xAA00, 0xCCBB, 0xEEDD]) ); } @@ -1224,51 +1225,57 @@ mod tests { #[test] fn read_holding_registers() { let bytes = Bytes::from(vec![3, 0x04, 0xAA, 0x00, 0x11, 0x11]); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::ReadHoldingRegisters(vec![0xAA00, 0x1111])); + let response = Response::try_from(bytes).unwrap(); + assert_eq!( + response, + Response::ReadHoldingRegisters(vec![0xAA00, 0x1111]) + ); } #[test] fn write_single_register() { let bytes = Bytes::from(vec![6, 0x00, 0x07, 0xAB, 0xCD]); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::WriteSingleRegister(0x07, 0xABCD)); + let response = Response::try_from(bytes).unwrap(); + assert_eq!(response, Response::WriteSingleRegister(0x07, 0xABCD)); } #[test] fn write_multiple_registers() { let bytes = Bytes::from(vec![0x10, 0x00, 0x06, 0x00, 0x02]); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::WriteMultipleRegisters(0x06, 2)); + let response = Response::try_from(bytes).unwrap(); + assert_eq!(response, Response::WriteMultipleRegisters(0x06, 2)); } #[test] fn report_server_id() { let bytes = Bytes::from(vec![0x11, 0x04, 0x042, 0xFF, 0x10, 0x20]); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::ReportServerId(0x42, true, vec![0x10, 0x20])); + let response = Response::try_from(bytes).unwrap(); + assert_eq!( + response, + Response::ReportServerId(0x42, true, vec![0x10, 0x20]) + ); } #[test] fn masked_write_register() { let bytes = Bytes::from(vec![0x16, 0x00, 0x06, 0x80, 0x01, 0x40, 0x02]); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::MaskWriteRegister(6, 0x8001, 0x4002)); + let response = Response::try_from(bytes).unwrap(); + assert_eq!(response, Response::MaskWriteRegister(6, 0x8001, 0x4002)); } #[test] fn read_write_multiple_registers() { let bytes = Bytes::from(vec![0x17, 0x02, 0x12, 0x34]); - let rsp = Response::try_from(bytes).unwrap(); - assert_eq!(rsp, Response::ReadWriteMultipleRegisters(vec![0x1234])); + let response = Response::try_from(bytes).unwrap(); + assert_eq!(response, Response::ReadWriteMultipleRegisters(vec![0x1234])); } #[test] fn custom() { let bytes = Bytes::from(vec![0x55, 0xCC, 0x88, 0xAA, 0xFF]); - let rsp = Response::try_from(bytes).unwrap(); + let response = Response::try_from(bytes).unwrap(); assert_eq!( - rsp, + response, Response::Custom(0x55, Bytes::from_static(&[0xCC, 0x88, 0xAA, 0xFF])) ); }