From d0950d37b2f24e2c203fd632be4c9a736395f59c Mon Sep 17 00:00:00 2001 From: Elie ROUDNINSKI Date: Thu, 10 Sep 2026 16:28:12 +0200 Subject: [PATCH] Validate registered identifiers --- engine/src/lib.rs | 4 +- engine/src/scheme.rs | 198 +++++++++++++++++++++++++++++++++++-------- ffi/src/lib.rs | 13 +++ 3 files changed, 177 insertions(+), 38 deletions(-) diff --git a/engine/src/lib.rs b/engine/src/lib.rs index 8ea557f6..527bb5e2 100644 --- a/engine/src/lib.rs +++ b/engine/src/lib.rs @@ -118,8 +118,8 @@ pub use self::rhs_types::{ }; pub use self::scheme::{ Field, FieldIndex, FieldRedefinitionError, FieldRef, Function, FunctionRedefinitionError, - FunctionRef, IdentifierRedefinitionError, IndexAccessError, List, ListRef, Scheme, - SchemeBuilder, SchemeMismatchError, UnknownFieldError, + FunctionRef, IdentifierRedefinitionError, IdentifierRegistrationError, IndexAccessError, List, + ListRef, Scheme, SchemeBuilder, SchemeMismatchError, UnknownFieldError, }; pub use self::types::{ CompoundType, ExpectedType, ExpectedTypeList, GetType, LhsValue, LiteralSet, LiteralValue, diff --git a/engine/src/scheme.rs b/engine/src/scheme.rs index 854ef386..d491e942 100644 --- a/engine/src/scheme.rs +++ b/engine/src/scheme.rs @@ -426,22 +426,8 @@ pub(crate) enum Identifier<'s> { } impl<'i, 's> LexWith<'i, &'s Scheme> for Identifier<'s> { - fn lex_with(mut input: &'i str, scheme: &'s Scheme) -> LexResult<'i, Self> { - let initial_input = input; - - loop { - input = take_while(input, "identifier character", |c| { - c.is_ascii_alphanumeric() || c == '_' - })? - .1; - - match expect(input, ".") { - Ok(rest) => input = rest, - Err(_) => break, - }; - } - - let name = span(initial_input, input); + fn lex_with(input: &'i str, scheme: &'s Scheme) -> LexResult<'i, Self> { + let (name, input) = lex_identifier_name(input)?; let field = scheme .get(name) @@ -451,6 +437,25 @@ impl<'i, 's> LexWith<'i, &'s Scheme> for Identifier<'s> { } } +#[inline] +fn lex_identifier_name(mut input: &str) -> LexResult<'_, &str> { + let initial_input = input; + + loop { + input = take_while(input, "identifier character", |c| { + c.is_ascii_alphanumeric() || c == '_' + })? + .1; + + match expect(input, ".") { + Ok(rest) => input = rest, + Err(_) => break, + }; + } + + Ok((span(initial_input, input), input)) +} + /// An error that occurs if an unregistered field name was queried from a /// [`Scheme`](struct@Scheme). #[derive(Debug, PartialEq, Eq, Error)] @@ -485,6 +490,28 @@ pub enum IdentifierRedefinitionError { Function(#[source] FunctionRedefinitionError), } +/// An error that occurs when registering a field or function identifier. +#[derive(Debug, PartialEq, Eq, Error)] +pub enum IdentifierRegistrationError { + /// The identifier does not follow the syntax accepted for registration. + #[error("invalid identifier {0}")] + InvalidIdentifier(String), + + /// The identifier is already registered as a field or function. + #[error("{0}")] + Redefinition(#[from] IdentifierRedefinitionError), +} + +fn is_valid_identifier(name: &str) -> bool { + matches!(lex_identifier_name(name), Ok((_, ""))) + && matches!(name.as_bytes().first(), Some(b) if b.is_ascii_alphabetic() || *b == b'_') + && name.split('.').all(|segment| { + segment + .bytes() + .any(|b| b.is_ascii_alphabetic() || b == b'_') + }) +} + #[derive(Clone, Copy, Debug)] enum SchemeItem { Field(usize), @@ -647,16 +674,23 @@ impl SchemeBuilder { name: Arc, ty: Type, optional: bool, - ) -> Result<(), IdentifierRedefinitionError> { + ) -> Result<(), IdentifierRegistrationError> { + if !is_valid_identifier(&name) { + return Err(IdentifierRegistrationError::InvalidIdentifier( + name.to_string(), + )); + } + match self.items.entry(name) { - Entry::Occupied(entry) => match entry.get() { - SchemeItem::Field(_) => Err(IdentifierRedefinitionError::Field( - FieldRedefinitionError(entry.key().to_string()), + Entry::Occupied(entry) => Err(match entry.get() { + SchemeItem::Field(_) => IdentifierRedefinitionError::Field(FieldRedefinitionError( + entry.key().to_string(), )), - SchemeItem::Function(_) => Err(IdentifierRedefinitionError::Function( + SchemeItem::Function(_) => IdentifierRedefinitionError::Function( FunctionRedefinitionError(entry.key().to_string()), - )), - }, + ), + } + .into()), Entry::Vacant(entry) => { let index = self.fields.len(); self.fields.push(FieldDefinition { @@ -675,7 +709,7 @@ impl SchemeBuilder { &mut self, name: N, ty: Type, - ) -> Result<(), IdentifierRedefinitionError> { + ) -> Result<(), IdentifierRegistrationError> { self.add_field_full(name.as_ref().into(), ty, false) } @@ -684,7 +718,7 @@ impl SchemeBuilder { &mut self, name: N, ty: Type, - ) -> Result<(), IdentifierRedefinitionError> { + ) -> Result<(), IdentifierRegistrationError> { self.add_field_full(name.as_ref().into(), ty, true) } @@ -693,16 +727,24 @@ impl SchemeBuilder { &mut self, name: N, function: impl FunctionDefinition + 'static, - ) -> Result<(), IdentifierRedefinitionError> { - match self.items.entry(name.as_ref().into()) { - Entry::Occupied(entry) => match entry.get() { - SchemeItem::Field(_) => Err(IdentifierRedefinitionError::Field( - FieldRedefinitionError(entry.key().to_string()), + ) -> Result<(), IdentifierRegistrationError> { + let name = name.as_ref(); + if !is_valid_identifier(name) { + return Err(IdentifierRegistrationError::InvalidIdentifier( + name.to_string(), + )); + } + + match self.items.entry(name.into()) { + Entry::Occupied(entry) => Err(match entry.get() { + SchemeItem::Field(_) => IdentifierRedefinitionError::Field(FieldRedefinitionError( + entry.key().to_string(), )), - SchemeItem::Function(_) => Err(IdentifierRedefinitionError::Function( + SchemeItem::Function(_) => IdentifierRedefinitionError::Function( FunctionRedefinitionError(entry.key().to_string()), - )), - }, + ), + } + .into()), Entry::Vacant(entry) => { let index = self.functions.len(); self.functions @@ -1854,10 +1896,83 @@ fn test_field_type_override() { assert_eq!( builder.add_field("foo", Type::Bytes), - Err(IdentifierRedefinitionError::Field(FieldRedefinitionError( - "foo".into() - ))) + Err(IdentifierRegistrationError::Redefinition( + IdentifierRedefinitionError::Field(FieldRedefinitionError("foo".into())) + )) + ); +} + +#[test] +fn test_identifier_registration() { + use crate::ConcatFunction; + + let mut builder = SchemeBuilder::new(); + let valid_field_names = [ + "_", + "field", + "Field_0", + "http._request", + "a.b2._c3", + "a.b2c", + "a2b.c", + "a2b", + "cf.response.1xxx_code", + ]; + for name in valid_field_names { + assert_eq!(builder.add_field(name, Type::Bytes), Ok(())); + } + assert_eq!( + builder.add_optional_field("optional.field", Type::Int), + Ok(()) + ); + assert_eq!( + builder.add_function("function.name", ConcatFunction::new()), + Ok(()) + ); + + for name in [ + "", + ".field", + "field.", + "field..name", + "0field", + "123", + "192.0.2.1", + "a.1", + "a.123", + "a.1.2", + "f-ield", + "féield", + ] { + assert_eq!( + builder.add_field(name, Type::Bytes), + Err(IdentifierRegistrationError::InvalidIdentifier(name.into())) + ); + } + + assert_eq!( + builder.add_optional_field("optional.", Type::Int), + Err(IdentifierRegistrationError::InvalidIdentifier( + "optional.".into() + )) + ); + assert_eq!( + builder.add_function("1function", ConcatFunction::new()), + Err(IdentifierRegistrationError::InvalidIdentifier( + "1function".into() + )) ); + + let scheme = builder.build(); + for name in valid_field_names.into_iter().chain(["optional.field"]) { + let (field, rest) = FieldRef::lex_with(name, &scheme).unwrap(); + assert_eq!(field.name(), name); + assert_eq!(rest, ""); + } + + let (function, rest) = FunctionRef::lex_with("function.name", &scheme).unwrap(); + assert_eq!(function.name(), "function.name"); + assert_eq!(rest, ""); } #[test] @@ -1920,6 +2035,17 @@ fn test_scheme_json_serialization() { assert_eq!(scheme.inner.fields, new_scheme.inner.fields); } +#[test] +fn test_scheme_json_rejects_invalid_identifier() { + let scheme = Scheme! { valid: Int }.build(); + let json = serde_json::to_string(&scheme) + .unwrap() + .replace("\"valid\"", "\"1invalid\""); + + let err = serde_json::from_str::(&json).unwrap_err(); + assert!(err.to_string().contains("invalid identifier 1invalid")); +} + #[test] fn test_nil_not_equal_behavior_true() { use crate::{Array, ExecutionContext, Map}; diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs index 7fe3ecbb..93994ad9 100644 --- a/ffi/src/lib.rs +++ b/ffi/src/lib.rs @@ -888,6 +888,19 @@ mod ffi_test { wirefilter_build_scheme(builder) } + #[test] + fn reject_invalid_field_identifier() { + let mut builder = wirefilter_create_scheme_builder(); + let name = "2http.host"; + + assert!(!wirefilter_add_type_field_to_scheme( + &mut builder, + name.as_ptr().cast(), + name.len(), + Type::Bytes.into(), + )); + } + fn create_execution_context<'e, 's: 'e>(scheme: &'s Scheme) -> Box> { let mut exec_context = wirefilter_create_execution_context(scheme); let invalid_key = &b"\xc3\x28"[..];