Improve handling of filter values containing "+" and "%" - #143
Merged
apdavison merged 1 commit intoSep 15, 2026
Merged
Conversation
In the master branch, get_filter_value() truncates string filter values at the first "+" (or raised ValueError if it appeared in the first three characters), to work around a KG bug affecting filter values sent as request parameters for stored queries. This change removes the workaround since fairgraph no longer uses stored queries, it writes fresh queries each time that include the filter values, and in this case filters containing "+" are handled correctly. This means that we can now match values such as "C++", email addresses with a "+" suffix, or timestamps with a UTC offset. This also removes the exemptions for date/datetime properties and for Regex values, which existed only to bypass the workaround, and replaces the Regex-specific test (and the timezone-aware datetime test) with a single test that filter values containing "+" are passed through unchanged. The underlying bug still affects filter values sent as request parameters: the KG decodes parameter values twice, once by Spring and again in DataQueryBuilder.createAqlForFilter() in marmotgraph-core, so a "+" is received as a space and a "%" causes an error or is decoded together with the following characters. The bug is absent from the v4 branch of marmotgraph-core. fairgraph no longer passes filter values as parameters, but KGClient.query() still accepts a `filter` dict, so it now raises ValueError if a filter value contains "+" or "%", rather than silently returning the wrong results. A live test checks the current KG behaviour, so that we will know when v4 has been deployed and this check can be removed.
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.
In the master branch,
get_filter_value()truncates string filter values at the first "+" (or raisedValueErrorif it appeared in the first three characters), to work around a KG bug affecting filter values sent as request parameters for stored queries.This change removes the workaround, since fairgraph no longer uses stored queries, it writes fresh queries each time that include the filter values, and in this case filters containing "+" are handled correctly. This means that we can now match values such as "C++", email addresses with a "+" suffix, or timestamps with a UTC offset.
This also removes the exemptions for date/datetime properties and for
Regexvalues, which existed only to bypass the workaround, and replaces the Regex-specific test (and the timezone-aware datetime test) with a single test that filter values containing "+" are passed through unchanged.The underlying bug still affects filter values sent as request parameters: the KG decodes parameter values twice, once by Spring and again in
DataQueryBuilder.createAqlForFilter()in marmotgraph-core, so a "+" is received as a space and a "%" causes an error or is decoded together with the following characters. The bug is absent from the v4 branch of marmotgraph-core. fairgraph no longer passes filter values as parameters, butKGClient.query()still accepts afilterdict, so it now raisesValueErrorif a filter value contains "+" or "%", rather than silently returning the wrong results. A live test checks the current KG behaviour, so that we will know when v4 has been deployed and this check can be removed.