Skip to content

[#161] Keep every match of negated, multi-valued and numeric XML search filters - #203

Merged
maximthomas merged 10 commits into
OpenIdentityPlatform:masterfrom
maximthomas:issues/161-xml-filter-matches
Oct 28, 2026
Merged

maximthomas merged 10 commits into
OpenIdentityPlatform:masterfrom
maximthomas:issues/161-xml-filter-matches

Conversation

@maximthomas

@maximthomas maximthomas commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Closes #161.

By default the framework applies the filter again to every object the XML connector returns. That drops wrong matches but cannot add the ones the generated XQuery missed, so every miss was lost silently. These filters missed matches or failed:

  • Negated ordering comparisons. not(a > v) became a < v and not(a < v) became a > v, so an entry equal to v was lost. not(a >= v) and not(a <= v) had the opposite swap and returned entries equal to v. Each negation now emits its exact complement: <=, <, >= and >.

  • startsWith and endsWith on multi-valued attributes. fn:starts-with($x/ri:email, 'y') takes a single item, so one entry with two emails failed every search with that filter or its negation (XPTY0004). The filter is now a predicate on each value, $x/ri:email[fn:starts-with(., 'y')], negated as fn:not($x/ri:email[...]). The framework never matches a multi-valued attribute for these filters, so it drops the extra entries this returns.

  • Numbers compared as strings. A Number value of >, >=, < and <= was quoted, so age > 9 missed age 10. Now the XQuery keeps every value it does not exclude: age > 9 is $x/ri:age[. != '' and not(number(.) <= xs:double('9'))].

    Filter value Excluded for > and >= (mirrored for < and <=) Why no match is lost
    Integer and Byte; Long and BigInteger of magnitude below 2^53 <= and < The bound is a double, and no other integer reads as it.
    Any other Number: Double, Float, BigDecimal, larger integers < for both Reading a decimal as a double is monotonic. -0.0 equals 0.0.
    • Text that Java reads and XPath does not, such as Infinity or non-Latin digits, reads as NaN, and not(NaN <= 9) holds, so the entry is kept. Text that Java cannot read either, such as abc, is kept too, and reading it back fails the search, as it fails a search without a filter. Before, age < 10 skipped such an entry.
    • L is the value's own toString(), with Infinity written as INF.
    • For a Float, L is instead the lower (upper) end of the decimals that Java reads as that float. XPath reads a stored float as a double, and its text need not be the one the current JDK writes: before JDK 19, Float.toString wrote some floats with more digits, such as 3.3565272E7 for what it now writes as 3.356527E7.
    • Both rows add only text that XPath cannot read as a number, and the second row also values that read as equal to the bound.
    • number() rather than a cast keeps the empty elements that create writes for unset attributes from failing the search. . != '' keeps them out.
    • = and != stay string comparisons.
  • An & in the value. An XQuery string literal decodes entity references, and only ' was escaped. equalTo, startsWith or > on AT&T failed the search ("No closing ';' found for entity or character reference"). AT&amp;T was read as AT&T, so equalTo returned the entry stored as AT&T instead of the one stored as AT&amp;T, and its negation lost the AT&T entry. Every literal now writes & as &amp;. The __UID__ and __NAME__ index lookups read the raw value and are unchanged.

  • contains was not escaped. The value went between quotes as it was, so contains(firstname, "O'B") failed with "Unmatched quote in expression", and an & failed as above. contains now builds $x/ri:firstname[fn:contains(., 'v')] and its fn:not(...) with the same escaped literal as startsWith, and ContainsQuery is removed.

  • containsAllValues with one value. It became a substring contains, so not(containsAllValues(email, ["x"])) lost an entry whose email is x1, and a value that is not a String, such as [9], failed with "Value must be a string!". One value is now an equality, $x/ri:email = 'x', as each of several values already was. A null value failed with an IllegalArgumentException or a NullPointerException; it is now left to the framework, as an empty list already was.

  • equalTo with other than one value. AttributeUtil.getSingleValue rejected equalTo(email, ["x2", "y2"]), and no value or a null value failed with a NullPointerException. The framework compares the whole list of values, which an XQuery comparison does not; such a filter now translates to null, so the connector returns every entry and the framework selects from them.

Tests

XMLFilterSearchTests (new) searches a five-entry file through the facade with setEnableFilteredResultsHandler(false), so every miss shows. For each of 49 filters it checks that the result contains every entry the framework's own filter (FilteredResultsHandlerVisitor) selects, and that it equals the expected list. The filters cover:

  • the four negations;
  • startsWith and endsWith, and their negations, on a two-valued attribute;
  • every comparison with and without not for an int;
  • > and < for an Integer stored in Arabic-Indic digits, which XPath reads as NaN;
  • the 2^53 boundaries for Long and BigInteger, including Long.MIN_VALUE and -2^53;
  • Infinity, -0.0 and <= for a Double;
  • >= and <= for a Float stored as two decimals that are not Float.toString's text but read as it;
  • a BigDecimal bound;
  • first names AT&T, AT&amp;T and O'B: & in equalTo and its negation, startsWith and >, and ' and & in contains and its negation;
  • containsAllValues with one String and its negation, with one Integer, and with a null value alone or beside another;
  • equalTo and its negation with two values, and equalTo with no value and with a null value.

XMLFilterTranslatorTests pins the generated expressions, including a Byte bound and = with a Double, the ends of the rounding interval of 0.125f (at a power of two the gap to the float below is half the gap to the float above), &amp; in a comparison and in a function literal, and contains with a quote.

Without the fixes, 45 of the 49 filters and five of the six new XMLFilterTranslatorTests methods fail; the sixth calls roundingEdge, which does not exist there. The remaining four filters pin choices inside the new code. Each of these mutants fails at least one test:

  • Math.abs on Long.MIN_VALUE;
  • the signed bitLength() of -2^53;
  • an inclusive 2^53 threshold;
  • a Long or BigInteger bound never compared exactly;
  • a Byte bound not compared exactly;
  • a Double or BigDecimal bound compared exactly;
  • an exact bound written as number(.) > 9, which drops text XPath reads as NaN;
  • an exact bound never strict, or strict for >= and <= too;
  • a Float bound used as it is, with its ends swapped, half an ulp away, or at the neighbouring floats;
  • no INF;
  • no . != '';
  • a wrong operator in the <= branch alone;
  • = with a Number compared as a number;
  • no &amp; in comparison literals, in function literals, or in either;
  • contains without escaping;
  • one containsAllValues value that ignores not;
  • no null check in containsAllValues, or one on the first value only;
  • an equalTo that drops any of its checks: a null list, a size other than one, a null value.

Not covered

  • A hand-edited Float beyond Float.MAX_VALUE, such as 3.5E38, which Java reads as Infinity: >= Float.POSITIVE_INFINITY loses it.
  • = and != on a Number compare text, so an entry whose text is not the value's toString() is lost: non-Latin digits, or a Float or Double written by a JDK older than 19 with a different text for the same value (JDK 17 wrote 1.0E23 as 9.999999999999999E22).
  • String order: Java compares UTF-16 code units, XPath compares code points.
  • Text split by a comment or a CDATA section.
  • enableCaseInsensitiveFilter=true.
  • QueryImpl.and and QueryImpl.or add no parentheses. This only returns extra entries.
  • containsAllValues with several values joins the equalities with or, so an entry that has only one of them is returned too. This only returns extra entries.

… search filters

not(a > v) became a < v and not(a < v) became a > v, so an entry equal to v was lost. not(a >= v) and not(a <= v) had the opposite swap and returned entries equal to v. Each negation now emits the exact complement: <=, <, >= and >.
…With filters

fn:starts-with($x/ri:email, 'y') takes a single item, so one entry with two emails failed every search with that filter or its negation (XPTY0004). The filter is now a predicate on each value, $x/ri:email[fn:starts-with(., 'y')], negated with fn:not() around it.
…s numbers

A Number value of >, >=, < and <= was compared as a quoted string, so age > 9 missed age 10. An integer bound of magnitude below 2^53 is now compared directly, $x/ri:age[number(.) > 9]. Any other number gives a non-strict form that keeps every value the framework selects: $x/ri:n[. != '' and not(number(.) < xs:double('L'))] for > and >=, mirrored for < and <=. number() rather than a cast keeps the empty elements of unset attributes from failing the search.
…l that reads as it

XPath reads a stored float as a double, and before JDK 19 Float.toString wrote some floats with more digits (3.3565272E7, now 3.356527E7). With the bound's own toString() as the literal, >= and <= lost an entry stored with the other text. The literal is now the lower or upper end of the decimals Java reads as the float. Also pins the non-strict <= branch.
@maximthomas maximthomas added bug Something isn't working java Pull requests that update java code tests Test additions or fixes connector:xml XML connector labels Oct 23, 2026
…ger XML filter bound

The exact form of an integer bound, $x/ri:age[number(.) > 9], dropped stored text that Java reads and XPath reads as NaN: non-Latin digits in an int attribute, or Infinity in a double attribute. An integer bound of magnitude below 2^53 now uses the same negated form as any other bound and only keeps its operator strict: $x/ri:age[. != '' and not(number(.) <= xs:double('9'))]. Text that Java cannot read either, such as abc, is kept too, so reading it back fails the search, as it fails a search without a filter.
…Number equality in XML filter tests

A Byte bound is compared exactly, like an Integer. roundingEdge is pinned at 0.125f, a power of two, where the gap to the float below is half the gap to the float above, so a symmetric interval or one out to the neighbouring floats fails. = with a Double still compares the quoted text.
@maximthomas
maximthomas marked this pull request as ready for review October 23, 2026 17:53

@vharseko vharseko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the translator against the framework's own filters and against Saxon-HE 9.4.0.7, the version the bundle embeds. I found no regression:

  • the four negations are exact complements;
  • the per-value starts-with / ends-with predicate is correct;
  • the numeric bounds never exclude a value the framework selects. This includes the 2^53 cut-off with Long.MIN_VALUE and -2^53, the Float rounding edges, -0.0 and INF.

CI is green.

The points below already exist on master; this PR does not introduce them. They belong to the same class as #161: a filter that fails or loses matches. The first two are small and sit on helpers this PR builds on, so they could be folded in here. The rest could go to a follow-up issue. Your call.

  1. escape() does not escape & (L292). XQuery string literals decode entity references:

    • startsWith(lastname, "AT&T"), lastname = "AT&T" and lastname > "AT&T" all produce the literal 'AT&T'. Saxon rejects it with XPST0003 No closing ';' found for entity or character reference, and the whole search fails with a ConnectorException.
    • lastname = "AT&amp;T" produces 'AT&amp;T'. That literal matches the entry whose text is AT&T and loses the entry whose text is AT&amp;T.

    Fix: "'" + value.replace("&", "&amp;").replace("'", "''") + "'". The index path is not affected, because ComparisonQuery keeps the raw value separately.

  2. contains is not escaped at all (L143). ContainsQuery pastes the value between quotes as it is. So contains(lastname, "O'B") produces [contains(.,'O'B')], which fails with XPST0003 Unmatched quote in expression. A value containing & fails as in point 1. createPredicateQuery(prefixedName, createFunctionCondition("contains", value), not) builds the same $x/name[...] / fn:not(...) shape with escaping, and ContainsQuery can then be deleted.

  3. A single-value containsAllValues becomes a substring contains (L98). This causes two problems:

    • not(containsAllValues(email, ["v"])) produces fn:not($x/ri:email[contains(.,'v')]), which loses an entry with email = ["vx"]. The framework selects that entry, because ["vx"].containsAll(["v"]) is false.
    • A non-String value, such as containsAllValues(age, [5]), throws IllegalArgumentException("Value must be a string!"), because ContainsFilter is a StringFilter.

    Building an EqualsFilter from toString(), as the multi-value branch already does, fixes both.

  4. equals on a multi-valued filter attribute throws (L152). AttributeUtil.getSingleValue rejects equals(email, ["x2", "y2"]) with IllegalArgumentException, so the search fails. Returning null in that case would keep the match: the framework then filters every entry.

Nit: createFunctionCondition (L283) builds a FunctionQuery only to format fn:name(., literal). After this PR, the not branch of FunctionQuery has no production caller; only testFunctionQueryExpressionWhereNotIsTrue uses it. It is not worth a change on its own.

An XQuery string literal decodes entity references, and escape() doubled only the quote. A filter value with an & then failed the whole search: equalTo, startsWith or greaterThan on AT&T was rejected with "No closing ';' found for entity or character reference". AT&amp;T was read as AT&T, so equalTo matched the entry stored as AT&T instead of the one stored as AT&amp;T, and its negation lost the AT&T entry. The literal now writes & as &amp;. The __UID__ and __NAME__ index lookups read the raw value and are unchanged.
ContainsQuery wrote the value between quotes as it was, so contains(firstname, "O'B") failed the search with "Unmatched quote in expression", an & failed it as in the other filters, and &amp; was read as &. contains now builds $x/ri:firstname[fn:contains(., 'v')] and its fn:not(...) through the same predicate and escaped literal as startsWith and endsWith, and ContainsQuery, which has no other caller, is removed.
…equality in XML search filters

A containsAllValues filter with one value became a substring contains: not(containsAllValues(email, ["x"])) produced fn:not($x/ri:email[contains(., 'x')]) and lost the entries whose email is x1, which the framework selects, and a value that is not a String, such as containsAllValues(age, [9]), failed with "Value must be a string!". One value is now an equality, as each of several values already was: $x/ri:email = 'x', negated $x/ri:email != 'x'. A null value made the search fail with an IllegalArgumentException or a NullPointerException; it now leaves the filter to the framework, as no value already did.
… one value to the framework

equalTo read its value with AttributeUtil.getSingleValue, so a filter with several values, such as equalTo(email, ["x2", "y2"]), failed the search with "The email attribute is not single value attribute.", and one with no value or a null value failed with a NullPointerException. The framework compares the whole list of values, which an XQuery comparison does not; such a filter now translates to null, so the connector returns every entry and the framework selects from them, as it does for an empty containsAllValues.
@maximthomas

Copy link
Copy Markdown
Contributor Author

Review 5665044757: all four points are folded into this PR, one commit each (ce81d9cb..fadb4a48), together with the empty and null values of the same two methods. The approval predates these commits.

  1. 9e3664eb: escape() writes & as &amp;, as proposed. One more case besides yours: not(equalTo(firstname, "AT&amp;T")) lost the entry stored as AT&T. A row pins it.
  2. 806a4a0e: contains goes through createPredicateQuery(prefixedName, createFunctionCondition("contains", value), not), and ContainsQuery is deleted.
  3. 79ed89c2: one value is now an EqualsFilter built from toString(). A null value, alone or beside another, used to fail with IllegalArgumentException / NullPointerException. It now returns null, as an empty list already did. ContainsAllValuesFilter itself rejects a null list.
  4. fadb4a48: createEqualsExpression returns null unless the filter has exactly one non-null value. This also covers equalTo(email) with no value (getValue() is null) and equalTo(email, null), which both failed with a NullPointerException.

Nit: left as it is. contains is now a second production caller of createFunctionCondition, and the not branch of FunctionQuery still has none.

Tests:

  • XMLFilterSearchTests has 17 new rows, 49 in all. The fixture adds the first names AT&T, AT&amp;T and O'B.
  • XMLFilterTranslatorTests has two new methods.
  • At ce81d9cb, exactly these 19 tests fail.
  • Each of 10 mutants fails at least one test: one per escape site, the contains escaping, not in the single-value branch, and each condition of both null checks.
  • Module: 321 unit tests + 1 IT, green on JDK 26 and 11.0.25.

The PR body is updated: new bullets in the filter list, Tests and the mutant list, and one Not covered line. containsAllValues with several values still joins the equalities with or, which only returns extra entries.

@maximthomas
maximthomas requested a review from vharseko October 27, 2026 11:56

@vharseko vharseko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked ce81d9cb..fadb4a48 against the four points. All four hold as fixed, and I found no regression in the new code:

  1. 9e3664eb: every literal goes through escape() (createComparisonQuery and createFunctionCondition), and the numericCondition literals are number text. The __UID__ / __NAME__ index lookup still reads ComparisonQuery.getValue(), the raw value, not the literal.
  2. 806a4a0e: contains now builds the same $x/name[fn:contains(., literal)] / fn:not(...) shape as startsWith. Nothing else referenced ContainsQuery.
  3. 79ed89c2: the negation $x/ri:email != 'x' is false for an entry with no email, but the framework never selects such an entry either: FilteredResultsHandlerVisitor returns UNDEFINED for a missing attribute, and not keeps it UNDEFINED. Rows a4 and a5 pin this. The null check runs before any toString(), so the several-value branch never gets null from createEqualsExpression.
  4. fadb4a48: a null query falls through to entryIndex.all, and the framework selects from every entry.

The nit stays as it is, agreed.

Locally at fadb4a48 on JDK 21, all 321 unit tests pass, including the 49 XMLFilterSearchTests rows and the 26 XMLFilterTranslatorTests. CI was still queued when I checked.

@maximthomas
maximthomas merged commit 8fa25f0 into OpenIdentityPlatform:master Oct 28, 2026
15 checks passed
@maximthomas
maximthomas deleted the issues/161-xml-filter-matches branch October 28, 2026 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working connector:xml XML connector java Pull requests that update java code tests Test additions or fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XML connector: some search filters silently lose matches or fail

2 participants