Repository navigation
Conversation
Inside a quoted OQL literal, iTop's lexer accepts exactly two escape sequences, \' and \\ (the 'strval' rule of core/oql/oql-lexer.plex), and its parser unescapes the literal with stripslashes(). quoteString() also emitted \_ \% \" \n \r \t \0 \Z \b, MySQL's real_escape_string set, and the lexer rejects every one of them: a value containing any of those characters turns the query into an OQL syntax error. In practice, get-person-from-email fails for any address containing an underscore, such as john_doe@example.com. The JSON layer needs nothing from this method: the .json.twig templates are rendered with Twig's 'js' escaping strategy, which already produces valid JSON whatever the value. The tests now check the property itself: once quoted and wrapped in single quotes, a value must match the lexer's literal pattern, and stripslashes() must give it back unchanged.
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix matches the OQL literal contract and includes relevant regression coverage, with no unresolved findings.
No open issues 🎉
What changed in this PR
Fixes OQL query failures for values such as email addresses containing underscores by limiting quoteString() to supported escapes.
Changes:
- Escapes only apostrophes and backslashes.
- Adds regression tests for valid OQL literals and unchanged round-trip values.
| File | Description |
|---|---|
| tests/phpunit/ToolsFromTemplateTest.php | Updates escaping expectations and adds literal validation tests. |
| src/Tools/iTopRestTools.php | Removes unsupported escapes and documents the OQL/JSON boundary. |
Review effort: Balanced
This was referenced Oct 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #25.
quoteString()now escapes only'and\, the two escape sequences iTop's OQL lexer accepts inside a quoted literal. The JSON layer needs nothing from this method: the.json.twigtemplates are rendered with Twig'sjsescaping strategy.If escaping
_and%was meant to keep them from acting asLIKEwildcards: the tools usingquoteString()compare with=, where both are plain characters, and iTop's lexer offers no way to escape them inside a literal anyway, so a value containing them could not be looked up at all.The tests check the property itself: once quoted and wrapped in single quotes, a value must match the lexer's literal pattern, and
stripslashes()must give it back unchanged.