From e0e453c9c573520a1c341b56ffaebe0f5bcd41f0 Mon Sep 17 00:00:00 2001 From: Nicola Belluti Date: Thu, 1 Oct 2026 14:43:50 +0200 Subject: [PATCH] Cleaned up the protocol layer and metadata - request() now reads exactly the expected response length with read_exact instead of a single non-guaranteed fill_buf, fixing a latent bug where a response split across multiple TCP packets could be mistaken for malformed data; the per-method length checks are now redundant and were removed - sequence_number wraps explicitly via wrapping_add instead of relying on implicit (and debug-vs-release inconsistent) overflow behavior after 65536 requests - get_name validates UTF-8 strictly instead of silently lossy-decoding invalid bytes; on invalid data it now returns the FromUtf8Error itself (byte-level \0 trimming moved after the UTF-8 check), letting the caller choose to fail or fall back to a lossy rendering - removed the unused Default derive from Clock and Record - removed npm from the CI image (no JS/Node in this repo), fixed a typo in a CI step name - fixed the Cargo.toml license field to the actual SPDX identifier (AGPL-3.0-or-later, per the notice at the end of LICENSE) instead of the free-text "GNU AGPLv3.0" Co-Authored-By: Claude Sonnet 5 --- .gitea/workflows/check-format-and-test.yaml | 4 +- Cargo.toml | 2 +- README.md | 10 ++-- src/r701.rs | 65 ++++++++++----------- src/record.rs | 11 +--- 5 files changed, 41 insertions(+), 51 deletions(-) diff --git a/.gitea/workflows/check-format-and-test.yaml b/.gitea/workflows/check-format-and-test.yaml index f21e58b..8726574 100644 --- a/.gitea/workflows/check-format-and-test.yaml +++ b/.gitea/workflows/check-format-and-test.yaml @@ -11,7 +11,7 @@ jobs: run: | rustup component add clippy rustfmt && apk update && - apk add musl-dev git npm + apk add musl-dev git - name: Checkout the code uses: actions/checkout@v3 @@ -27,5 +27,5 @@ jobs: - name: Check if the code is formatted correctly run: cargo fmt --check - - name: Check if Clippy has someting to say + - name: Check if Clippy has something to say run: cargo clippy --all-targets diff --git a/Cargo.toml b/Cargo.toml index 904c2bb..202ea30 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "r701" version = "0.1.0" edition = "2021" -license = "GNU AGPLv3.0" +license = "AGPL-3.0-or-later" repository = "https://git.nicolabelluti.me/nicolabelluti/r701" [dependencies] diff --git a/README.md b/README.md index 5fb532f..34dafba 100644 --- a/README.md +++ b/README.md @@ -45,11 +45,11 @@ If you want to read about this reverse engineering attempt, check out println!("No\tMchn\tEnNo\t\tName\t\tMode\tIOMd\tDateTime\t"); while let Some(record) = record_stream.next().await { let record = record.unwrap(); - let name = r701 - .get_name(record.employee_id) - .await - .unwrap() - .unwrap_or(format!("user #{}", record.employee_id)); + let name = match r701.get_name(record.employee_id).await.unwrap() { + Some(Ok(name)) => name, + Some(Err(error)) => String::from_utf8_lossy(error.as_bytes()).into_owned(), + None => format!("user #{}", record.employee_id), + }; println!( "{:0>6}\t{}\t{:0>9}\t{: <10}\t{}\t{}\t{}", diff --git a/src/r701.rs b/src/r701.rs index 8256cbe..9aa1945 100644 --- a/src/r701.rs +++ b/src/r701.rs @@ -1,6 +1,6 @@ use crate::RecordIterator; use std::io::{Error, ErrorKind::InvalidData, Result}; -use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpStream, ToSocketAddrs}; use tokio::sync::Mutex; @@ -35,7 +35,7 @@ impl R701 { // The lock must stay held for the whole write+read round-trip, so that // concurrent callers can't interleave their requests on the wire #[allow(clippy::significant_drop_tightening)] - pub async fn request(&self, payload: &[u8; 12]) -> Result> { + pub async fn request(&self, payload: &[u8; 12], response_len: usize) -> Result> { let mut state = self.state.lock().await; // Create a blank request @@ -46,14 +46,16 @@ impl R701 { // Insert the sequence number request[14..].clone_from_slice(&state.sequence_number.to_le_bytes()); - state.sequence_number += 1; + state.sequence_number = state.sequence_number.wrapping_add(1); // Send the request state.tcp_stream.write_all(&request).await?; - // Create a buffer and return the response - let mut buffer = BufReader::new(&mut state.tcp_stream); - Ok(buffer.fill_buf().await?.to_vec()) + // Read exactly response_len bytes, looping over multiple reads if + // the response arrives split across several TCP packets + let mut buffer = vec![0u8; response_len]; + state.tcp_stream.read_exact(&mut buffer).await?; + Ok(buffer) } #[allow(clippy::missing_errors_doc)] @@ -61,7 +63,7 @@ impl R701 { // Create a request with a payload of `01 80 00 00 00 00 00 00 00 00 00 // 00` and get the response let response = self - .request(&[0x01, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]) + .request(&[0x01, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 10) .await?; // If the response is not `aa 55 01 01 00 00 00 00 00 00` then return an @@ -74,39 +76,38 @@ impl R701 { } #[allow(clippy::missing_errors_doc)] - pub async fn get_name(&self, id: u32) -> Result> { + pub async fn get_name( + &self, + id: u32, + ) -> Result>> { // Create a request with a payload of `01 c7 xx xx xx xx 00 00 00 00 14 // 00`, where `xx xx xx xx` is the employee ID in little endian let mut request = [0x01, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0x14, 0]; request[2..6].clone_from_slice(&id.to_le_bytes()); // Get the response - let response = self.request(&request).await?; + let response = self.request(&request, 34).await?; - // If the response length is right but the header is `01 00 00 00 00 00 - // 00 00` then the request is been succesful but the name was not found - if response.len() == 34 - && response[..12] == [0xaa, 0x55, 0x01, 0, 0, 0, 0, 0, 0, 0, 0x55, 0xaa] - { + // If the header is `01 00 00 00 00 00 00 00` then the request has + // been succesful but the name was not found + if response[..12] == [0xaa, 0x55, 0x01, 0, 0, 0, 0, 0, 0, 0, 0x55, 0xaa] { return Ok(None); } - // If one between the response length or the response header is wrong - // return an error - if response.len() != 34 - || response[..12] != [0xaa, 0x55, 0x01, 0x01, 0, 0, 0, 0, 0, 0, 0x55, 0xaa] - { + // If the response header is wrong return an error + if response[..12] != [0xaa, 0x55, 0x01, 0x01, 0, 0, 0, 0, 0, 0, 0x55, 0xaa] { return Err(Error::new(InvalidData, "Malformed response")); } // Get the name as a UTF-8 string and delete the `\0` at the end - let name = String::from_utf8_lossy(&response[12..22]) - .trim_end_matches('\0') - .to_string(); + let name = match String::from_utf8(response[12..22].to_vec()) { + Ok(name) => name.trim_end_matches('\0').to_string(), + Err(error) => return Ok(Some(Err(error))), + }; // Return None if the name is empty, else return the name wrapped into a // Some - Ok(Some(name).filter(|name| !name.is_empty())) + Ok(Some(name).filter(|name| !name.is_empty()).map(Ok)) } #[allow(clippy::missing_errors_doc)] @@ -114,15 +115,12 @@ impl R701 { // Create a request with a payload of `01 b4 08 00 00 00 00 00 ff ff 00 // 00` and get the response let response = self - .request(&[0x01, 0xb4, 0x08, 0, 0, 0, 0, 0, 0xff, 0xff, 0, 0]) + .request(&[0x01, 0xb4, 0x08, 0, 0, 0, 0, 0, 0xff, 0xff, 0, 0], 10) .await?; - // If one between the response length or the response header is wrong + // If one between the response header or the trailing bytes is wrong // return an error - if response.len() != 10 - || response[..4] != [0xaa, 0x55, 0x01, 0x01] - || response[6..] != [0u8; 4] - { + if response[..4] != [0xaa, 0x55, 0x01, 0x01] || response[6..] != [0u8; 4] { return Err(Error::new(InvalidData, "Malformed response")); } @@ -155,12 +153,11 @@ impl R701 { } // Get the response - let response = self.request(&request).await?; + let response = self.request(&request, 1038).await?; - // If one between the response length, the response header or the last - // two bits is wrong return an error - if response.len() != 1038 - || response[..12] != [0xaa, 0x55, 0x01, 0x01, 0, 0, 0, 0, 0, 0, 0x55, 0xaa] + // If one between the response header or the last two bits is wrong + // return an error + if response[..12] != [0xaa, 0x55, 0x01, 0x01, 0, 0, 0, 0, 0, 0, 0x55, 0xaa] || response[1036..] != [0, 0] { return Err(Error::new(InvalidData, "Malformed response")); diff --git a/src/record.rs b/src/record.rs index db4b50c..9c56996 100644 --- a/src/record.rs +++ b/src/record.rs @@ -2,20 +2,16 @@ use chrono::{DateTime, Local, TimeZone}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -#[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub enum Clock { - #[default] FirstIn, FirstOut, SecondIn, SecondOut, } -// unsafe impl Send for Clock {} -// unsafe impl Sync for Clock {} - -#[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub struct Record { pub employee_id: u32, @@ -23,9 +19,6 @@ pub struct Record { pub datetime: DateTime, } -// unsafe impl Send for Record {} -// unsafe impl Sync for Record {} - impl TryFrom<&[u8]> for Record { type Error = &'static str;