From 312a0306cb0e491c7ea5dbbf1a19137bac95e338 Mon Sep 17 00:00:00 2001 From: digua Date: Sun, 5 Jul 2026 22:24:55 +0800 Subject: [PATCH] fix(parser-native): harden native parser test edge cases --- .github/workflows/test.yml | 8 + apps/cli/src/query/parse.test.ts | 4 +- packages/parser-native/src/jsutil.rs | 34 +++- packages/parser-native/src/scanner.rs | 237 +++++++++++++++++++++----- packages/parser-native/src/weflow.rs | 54 +++++- 5 files changed, 282 insertions(+), 55 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index de663daf..bc95f767 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -59,6 +59,14 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + - name: Prepare better-sqlite3 for Node tests + run: | + pnpm --filter chatlab-cli run rebuild-sqlite + pnpm --filter chatlab-cli run ensure-native + + - name: Build MCP package for workspace imports + run: pnpm --filter chatlab-mcp build + - name: Rust unit tests (parser-native) working-directory: packages/parser-native run: cargo test diff --git a/apps/cli/src/query/parse.test.ts b/apps/cli/src/query/parse.test.ts index 30636b9f..86763d21 100644 --- a/apps/cli/src/query/parse.test.ts +++ b/apps/cli/src/query/parse.test.ts @@ -38,8 +38,8 @@ describe('parseTimeOptions', () => { }) it('accepts "YYYY-MM-DD HH:mm" and full ISO 8601', () => { - const range = parseTimeOptions({ since: '2026-06-01 08:30', until: '2026-06-01T10:00:00+08:00' }, NOW) - assert.equal(range.startTs, localEpoch(2026, 6, 1, 8, 30)) + const range = parseTimeOptions({ since: '2026-05-31 08:30', until: '2026-06-01T10:00:00+08:00' }, NOW) + assert.equal(range.startTs, localEpoch(2026, 5, 31, 8, 30)) assert.equal(range.endTs, Math.floor(new Date('2026-06-01T10:00:00+08:00').getTime() / 1000)) }) diff --git a/packages/parser-native/src/jsutil.rs b/packages/parser-native/src/jsutil.rs index b3a8ccae..eb581e81 100644 --- a/packages/parser-native/src/jsutil.rs +++ b/packages/parser-native/src/jsutil.rs @@ -10,12 +10,11 @@ pub fn extract_name_from_file_path(file_path: &str, fallback: &str) -> String { .file_name() .map(|n| n.to_string_lossy().to_string()) .unwrap_or_default(); - let stripped = - if basename.len() >= 5 && basename[basename.len() - 5..].eq_ignore_ascii_case(".json") { - &basename[..basename.len() - 5] - } else { - basename.as_str() - }; + let suffix_start = basename.len().saturating_sub(5); + let stripped = match basename.get(suffix_start..) { + Some(suffix) if suffix.eq_ignore_ascii_case(".json") => &basename[..suffix_start], + _ => basename.as_str(), + }; if stripped.is_empty() { fallback.to_string() } else { @@ -30,3 +29,26 @@ pub fn non_empty_str(value: Option<&Value>) -> Option<&str> { _ => None, } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn extract_name_keeps_multibyte_name_without_json_suffix() { + assert_eq!(extract_name_from_file_path("/tmp/测试", "fallback"), "测试"); + } + + #[test] + fn extract_name_strips_json_suffix_case_insensitively() { + assert_eq!( + extract_name_from_file_path("/tmp/测试.JSON", "fallback"), + "测试" + ); + } + + #[test] + fn extract_name_uses_fallback_for_empty_basename() { + assert_eq!(extract_name_from_file_path("", "fallback"), "fallback"); + } +} diff --git a/packages/parser-native/src/scanner.rs b/packages/parser-native/src/scanner.rs index 7db83c04..4631ed2a 100644 --- a/packages/parser-native/src/scanner.rs +++ b/packages/parser-native/src/scanner.rs @@ -8,6 +8,14 @@ use memchr::memchr2; +fn is_digit(byte: u8) -> bool { + byte.is_ascii_digit() +} + +fn is_hex_digit(byte: u8) -> bool { + byte.is_ascii_hexdigit() +} + #[derive(Debug)] pub struct ScanError { pub message: String, @@ -104,15 +112,36 @@ impl<'a> JsonScanner<'a> { match memchr2(b'"', b'\\', &self.buf[self.pos..]) { Some(offset) => { let at = self.pos + offset; + if let Some(control_offset) = + self.buf[self.pos..at].iter().position(|b| *b < 0x20) + { + return Err(ScanError::new( + "unescaped control character in string", + self.pos + control_offset, + )); + } if self.buf[at] == b'"' { self.pos = at + 1; return Ok(&self.buf[start..at]); } - // Escape sequence: skip backslash + escaped byte. if at + 1 >= self.buf.len() { return Err(ScanError::new("unterminated escape sequence", at)); } - self.pos = at + 2; + match self.buf[at + 1] { + b'"' | b'\\' | b'/' | b'b' | b'f' | b'n' | b'r' | b't' => { + self.pos = at + 2; + } + b'u' => { + if at + 6 > self.buf.len() { + return Err(ScanError::new("unterminated unicode escape", at)); + } + if self.buf[at + 2..at + 6].iter().any(|b| !is_hex_digit(*b)) { + return Err(ScanError::new("invalid unicode escape", at)); + } + self.pos = at + 6; + } + _ => return Err(ScanError::new("invalid escape sequence", at)), + } } None => return Err(ScanError::new("unterminated string", start)), } @@ -127,21 +156,26 @@ impl<'a> JsonScanner<'a> { Some(b'"') => { self.scan_string()?; } - Some(b'{') | Some(b'[') => { - self.scan_container()?; + Some(b'{') => { + self.scan_object()?; + } + Some(b'[') => { + self.scan_array()?; + } + Some(b't') => { + self.scan_literal(b"true")?; + } + Some(b'f') => { + self.scan_literal(b"false")?; + } + Some(b'n') => { + self.scan_literal(b"null")?; + } + Some(b'-') | Some(b'0'..=b'9') => { + self.scan_number()?; } Some(_) => { - // Scalar: number / true / false / null. Scan until a structural - // delimiter or whitespace. - while self.pos < self.buf.len() { - match self.buf[self.pos] { - b',' | b'}' | b']' | b' ' | b'\t' | b'\n' | b'\r' => break, - _ => self.pos += 1, - } - } - if self.pos == start { - return Err(ScanError::new("expected a JSON value", start)); - } + return Err(ScanError::new("expected a JSON value", start)); } None => { return Err(ScanError::new( @@ -153,34 +187,123 @@ impl<'a> JsonScanner<'a> { Ok(&self.buf[start..self.pos]) } - /// Scan a `{...}` or `[...]` container assuming the opener is at `pos`. - /// - /// Structural bytes are walked one at a time (containers are mostly small), - /// while string contents — the bulk of large payloads such as base64 - /// avatars — are skipped via the memchr fast path in `scan_string`. - fn scan_container(&mut self) -> ScanResult<()> { + fn scan_literal(&mut self, literal: &[u8]) -> ScanResult<()> { + if self.buf.get(self.pos..self.pos + literal.len()) == Some(literal) { + self.pos += literal.len(); + Ok(()) + } else { + Err(ScanError::new("invalid JSON literal", self.pos)) + } + } + + fn scan_number(&mut self) -> ScanResult<()> { let start = self.pos; - let mut depth: usize = 0; - while self.pos < self.buf.len() { - match self.buf[self.pos] { - b'"' => { - self.scan_string()?; + self.consume(b'-'); + + match self.peek() { + Some(b'0') => { + self.pos += 1; + if self.peek().is_some_and(is_digit) { + return Err(ScanError::new("invalid leading zero in number", start)); } - b'{' | b'[' => { - depth += 1; + } + Some(b'1'..=b'9') => { + self.pos += 1; + while self.peek().is_some_and(is_digit) { self.pos += 1; } - b'}' | b']' => { - depth -= 1; - self.pos += 1; - if depth == 0 { - return Ok(()); - } - } - _ => self.pos += 1, + } + _ => return Err(ScanError::new("invalid JSON number", start)), + } + + if self.consume(b'.') { + if !self.peek().is_some_and(is_digit) { + return Err(ScanError::new("invalid JSON number fraction", self.pos)); + } + while self.peek().is_some_and(is_digit) { + self.pos += 1; } } - Err(ScanError::new("unterminated container", start)) + + if matches!(self.peek(), Some(b'e' | b'E')) { + self.pos += 1; + if matches!(self.peek(), Some(b'+' | b'-')) { + self.pos += 1; + } + if !self.peek().is_some_and(is_digit) { + return Err(ScanError::new("invalid JSON number exponent", self.pos)); + } + while self.peek().is_some_and(is_digit) { + self.pos += 1; + } + } + + Ok(()) + } + + fn scan_object(&mut self) -> ScanResult<()> { + self.expect(b'{')?; + self.skip_ws(); + if self.consume(b'}') { + return Ok(()); + } + + loop { + self.skip_ws(); + if !self.peek_is(b'"') { + return Err(ScanError::new("expected object key", self.pos)); + } + self.scan_string()?; + self.skip_ws(); + self.expect(b':')?; + self.scan_value()?; + self.skip_ws(); + + if self.consume(b',') { + self.skip_ws(); + if self.peek_is(b'}') { + return Err(ScanError::new("trailing comma in object", self.pos)); + } + continue; + } + self.expect(b'}')?; + return Ok(()); + } + } + + fn scan_array(&mut self) -> ScanResult<()> { + self.expect(b'[')?; + self.skip_ws(); + if self.consume(b']') { + return Ok(()); + } + + loop { + self.scan_value()?; + self.skip_ws(); + + if self.consume(b',') { + self.skip_ws(); + if self.peek_is(b']') { + return Err(ScanError::new("trailing comma in array", self.pos)); + } + continue; + } + self.expect(b']')?; + return Ok(()); + } + } +} + +fn expect_eof(scanner: &mut JsonScanner<'_>) -> ScanResult<()> { + scanner.skip_ws(); + if scanner.pos() == scanner.buf.len() { + Ok(()) + } else { + Err(ScanError::new( + "unexpected trailing characters after root JSON object", + scanner.pos(), + )) } } @@ -196,7 +319,7 @@ pub fn walk_top_level<'a>( scanner.expect(b'{')?; scanner.skip_ws(); if scanner.consume(b'}') { - return Ok(()); + return expect_eof(&mut scanner); } loop { @@ -212,7 +335,7 @@ pub fn walk_top_level<'a>( continue; } scanner.expect(b'}')?; - return Ok(()); + return expect_eof(&mut scanner); } } @@ -315,6 +438,24 @@ mod tests { assert!(collect_elements("[]").is_empty()); } + #[test] + fn errors_on_invalid_object_container_grammar() { + let result = walk_top_level(br#"{"chatlab": {bad}, "meta": {}}"#, |_, _| Ok(())); + assert!(result.is_err()); + } + + #[test] + fn errors_on_invalid_array_element_grammar() { + let result = walk_top_level(br#"{"messages": [{bad}]}"#, |_, _| Ok(())); + assert!(result.is_err()); + } + + #[test] + fn errors_on_invalid_scalar_token() { + let result = walk_top_level(br#"{"a": nope}"#, |_, _| Ok(())); + assert!(result.is_err()); + } + #[test] fn skips_bom() { let doc = "\u{FEFF}{\"messages\": [3]}"; @@ -323,6 +464,24 @@ mod tests { assert_eq!(collect_elements(&entries[0].1), vec!["3".to_string()]); } + #[test] + fn allows_trailing_whitespace_after_root_object() { + let entries = collect_entries("{\"a\": 1}\n\t "); + assert_eq!(entries, vec![("a".to_string(), "1".to_string())]); + } + + #[test] + fn errors_on_trailing_bytes_after_root_object() { + let result = walk_top_level(br#"{"a": 1} {"b": 2}"#, |_, _| Ok(())); + assert!(result.is_err()); + } + + #[test] + fn errors_on_trailing_bytes_after_empty_root_object() { + let result = walk_top_level(br#"{}[]"#, |_, _| Ok(())); + assert!(result.is_err()); + } + #[test] fn errors_on_truncated_document() { let result = walk_top_level(br#"{"a": [1, 2"#, |_, _| Ok(())); diff --git a/packages/parser-native/src/weflow.rs b/packages/parser-native/src/weflow.rs index 347700aa..49555230 100644 --- a/packages/parser-native/src/weflow.rs +++ b/packages/parser-native/src/weflow.rs @@ -198,14 +198,10 @@ pub fn parse_weflow( if let Some(raw) = messages_raw { let base_offset = raw.as_ptr() as usize - buf.as_ptr() as usize; for_each_array_element(raw, base_offset, |element, end_offset| { - let value: Value = match serde_json::from_slice(element) { - Ok(v) => v, - Err(_) => { - // Structurally valid JSON always deserializes into Value; this arm - // is unreachable in practice but must not abort the whole import. - return Ok(()); - } - }; + let value: Value = serde_json::from_slice(element).map_err(|err| ScanError { + message: format!("invalid message: {err}"), + offset: end_offset.saturating_sub(element.len()), + })?; let obj = match value.as_object() { Some(o) => o, None => return Ok(()), @@ -249,6 +245,13 @@ pub fn parse_weflow( let timestamp = match obj.get("createTime") { Some(Value::Number(n)) => n.as_f64(), + Some(Value::String(_)) => { + return Err(ScanError { + message: "unsupported createTime string; falling back to TS parser" + .to_string(), + offset: end_offset.saturating_sub(element.len()), + }); + } _ => None, }; @@ -395,6 +398,41 @@ mod tests { ); } + #[test] + fn string_create_time_errors_to_preserve_ts_fallback() { + let input = KernelInput { + primary_path: "/tmp/测试聊天.json".to_string(), + options_json: None, + }; + let result = parse_weflow( + br#"{"messages": [{"senderUsername": "wxid_a", "createTime": "1704164645", "content": "hi"}]}"#, + &input, + |_, _| {}, + ); + match result { + Ok(_) => panic!("string createTime must fall back to the TS parser"), + Err(err) => assert!( + err.message.contains("createTime string"), + "unexpected error: {}", + err.message + ), + } + } + + #[test] + fn malformed_message_object_errors_instead_of_skipping() { + let input = KernelInput { + primary_path: "/tmp/测试聊天.json".to_string(), + options_json: None, + }; + let result = parse_weflow( + br#"{"messages": [{bad}, {"senderUsername": "a", "createTime": 1}]}"#, + &input, + |_, _| {}, + ); + assert!(result.is_err()); + } + #[test] fn non_string_content_is_stringified() { let doc = r#"{"messages": [