From 2cede4cdd0ac8cd88350a770cec26e45c6a92263 Mon Sep 17 00:00:00 2001 From: Joe Wicentowski Date: Wed, 17 Jun 2026 17:44:18 -0400 Subject: [PATCH] [bugfix] map keys: compare by op:same-key, never coerce across type families A map{...} constructor builds a homogeneously-typed MapType that recorded a concrete keyType. MapType.get()/contains() then ran convert() on the lookup key, casting it to keyType (e.g. integer 12 -> string "12") before delegating to the underlying map. This conflated keys of different op:same-key families that share a lexical value: map{"12":"x"}(12) wrongly returned "x", and map{"2020-01-01":1}(xs:date("2020-01-01")) wrongly matched. It also conflated within the numeric family by lossy cast: map{1:1}(1.5) wrongly matched. Per XQuery 3.1 section 17.1, map keys compare via op:same-key, a type-group comparison with no key casting: the numeric family interchanges, the string family (xs:string/xs:anyURI/xs:untypedAtomic) interchanges, every other type matches only itself. convert() is now redundant. The underlying map's comparator already implements op:same-key (AbstractMapType.sameKey), and NumericValue/string-family hashCode are canonical, so cross-type-within-family pairs (xs:integer(5) / xs:double(5.0), xs:string / xs:anyURI) already hash into the same bucket. Removing convert() makes MapType behave like SingleKeyMapType, which already compares with sameKey directly. keyType is retained only for the getKeyType() API. XQTS misses this because qt3/qt4 place their cross-family distinctness tests on map:entry() (a MIXED-key map, where convert() was a no-op), never on the constructor form. New XQSuite tests (mapKeySameKey.xqm) cover the constructor form: cross-family distinctness plus within-family positives that must still interchange. Coupling: this closes the non-conformant serialization "bare-string backdoor". eXist serialization parameters are looked up by their exist: QName; a user could also pass them as the prefixed string "exist:expand-xincludes" only because convert() coerced the QName lookup to a string. With the fix, QName and string are distinct keys, so only the spec-aligned QName form is honored. The affected tests (serialize.xql, fnSerializeNewline.xqm, file module sync-serialize.xqm) are updated: the prefixed-string cases now assert the key is ignored, and the real functionality is retained via the QName form. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../exist/xquery/functions/map/MapType.java | 33 +--- .../src/test/xquery/maps/mapKeySameKey.xqm | 179 ++++++++++++++++++ .../xquery/xquery3/fnSerializeNewline.xqm | 15 +- .../src/test/xquery/xquery3/serialize.xql | 53 +++--- .../xquery/modules/file/sync-serialize.xqm | 4 +- 5 files changed, 226 insertions(+), 58 deletions(-) create mode 100644 exist-core/src/test/xquery/maps/mapKeySameKey.xqm diff --git a/exist-core/src/main/java/org/exist/xquery/functions/map/MapType.java b/exist-core/src/main/java/org/exist/xquery/functions/map/MapType.java index de724d4792b..6020046e9d4 100644 --- a/exist-core/src/main/java/org/exist/xquery/functions/map/MapType.java +++ b/exist-core/src/main/java/org/exist/xquery/functions/map/MapType.java @@ -66,6 +66,14 @@ public class MapType extends AbstractMapType { * then this is set to {@link #MIXED_KEY_TYPES}. *

* Uses integer values from {@link org.exist.xquery.value.Type}. + *

+ * This is reported via {@link #getKeyType()} but does NOT take part in key + * comparison. Lookups ({@link #get(AtomicValue)} / {@link #contains(AtomicValue)}) + * delegate directly to the underlying map, whose comparator implements + * {@code op:same-key} (see {@link AbstractMapType#sameKey}). Keys are compared + * by their op:same-key family, never coerced to {@code keyType} - a key from a + * different family that shares a lexical value (e.g. the string {@code "12"} + * and the integer {@code 12}) is correctly treated as distinct. */ private int keyType = UNKNOWN_KEY_TYPE; @@ -251,12 +259,7 @@ public void add(final AtomicValue key, final Sequence value) { } @Override - public Sequence get(AtomicValue key) { - key = convert(key); - if (key == null) { - return Sequence.EMPTY_SEQUENCE; - } - + public Sequence get(final AtomicValue key) { return map.get(key, Sequence.EMPTY_SEQUENCE); } @@ -267,12 +270,7 @@ public AbstractMapType put(final AtomicValue key, final Sequence value) { } @Override - public boolean contains(AtomicValue key) { - key = convert(key); - if (key == null) { - return false; - } - + public boolean contains(final AtomicValue key) { return map.contains(key); } @@ -375,17 +373,6 @@ private void setKeyType(final IMap newMap) { } } - private AtomicValue convert(final AtomicValue key) { - if (keyType != UNKNOWN_KEY_TYPE && keyType != MIXED_KEY_TYPES) { - try { - return key.convertTo(keyType); - } catch (final XPathException e) { - return null; - } - } - return key; - } - @Override public int getKeyType() { return keyType; diff --git a/exist-core/src/test/xquery/maps/mapKeySameKey.xqm b/exist-core/src/test/xquery/maps/mapKeySameKey.xqm new file mode 100644 index 00000000000..d3a74a65f45 --- /dev/null +++ b/exist-core/src/test/xquery/maps/mapKeySameKey.xqm @@ -0,0 +1,179 @@ +(: + : eXist-db Open Source Native XML Database + : Copyright (C) 2001 The eXist-db Authors + : + : info@exist-db.org + : http://www.exist-db.org + : + : This library is free software; you can redistribute it and/or + : modify it under the terms of the GNU Lesser General Public + : License as published by the Free Software Foundation; either + : version 2.1 of the License, or (at your option) any later version. + : + : This library is distributed in the hope that it will be useful, + : but WITHOUT ANY WARRANTY; without even the implied warranty of + : MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + : Lesser General Public License for more details. + : + : You should have received a copy of the GNU Lesser General Public + : License along with this library; if not, write to the Free Software + : Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + :) +xquery version "3.1"; + +(:~ + : Tests for map key comparison via op:same-key (XQuery 3.1 section 17.1). + : + : Map keys are compared with op:same-key, which is a TYPE-GROUP comparison: + : - the numeric family (xs:decimal/xs:integer/xs:float/xs:double) interchanges, + : - the string family (xs:string/xs:anyURI/xs:untypedAtomic) interchanges, + : - every other type matches only itself. + : There is NO key atomization or casting across groups. + : + : These tests deliberately use the map{...} CONSTRUCTOR form (which builds a + : homogeneously-typed map). The qt3/qt4 suites place their cross-family + : distinctness tests on map:entry() (a mixed-key map), so the constructor path + : was historically untested - and eXist used to conflate cross-family keys that + : shared a lexical value (e.g. map{"12":"x"}(12) wrongly returned "x"). + :) +module namespace skt="http://exist-db.org/xquery/test/maps/samekey"; + +import module namespace test="http://exist-db.org/xquery/xqsuite" at "resource:org/exist/xquery/lib/xqsuite/xqsuite.xql"; + +(: ----------------------------------------------------------------------- :) +(: Cross-family distinctness: keys of different op:same-key families that :) +(: share a lexical value must NOT match. :) +(: ----------------------------------------------------------------------- :) + +declare + %test:name("string key vs integer lookup are distinct (contains)") + %test:assertFalse +function skt:string-key-vs-integer-contains() { + map:contains(map { "12": "x" }, 12) +}; + +declare + %test:name("string key vs integer lookup are distinct (get)") + %test:assertEmpty +function skt:string-key-vs-integer-get() { + map:get(map { "12": "x" }, 12) +}; + +declare + %test:name("string key vs double lookup are distinct (contains)") + %test:assertFalse +function skt:string-key-vs-double-contains() { + map:contains(map { "5": 1 }, 5.0e0) +}; + +declare + %test:name("string key vs xs:date lookup are distinct (contains)") + %test:assertFalse +function skt:string-key-vs-date-contains() { + map:contains(map { "2020-01-01": 1 }, xs:date("2020-01-01")) +}; + +declare + %test:name("xs:date key vs string lookup are distinct (contains)") + %test:assertFalse +function skt:date-key-vs-string-contains() { + map:contains(map { xs:date("2020-01-01"): 1 }, "2020-01-01") +}; + +declare + %test:name("boolean key vs string lookup are distinct (contains)") + %test:assertFalse +function skt:boolean-key-vs-string-contains() { + map:contains(map { true(): "x" }, "true") +}; + +declare + %test:name("integer key vs string lookup are distinct (contains)") + %test:assertFalse +function skt:integer-key-vs-string-contains() { + map:contains(map { 12: "x" }, "12") +}; + +(: ----------------------------------------------------------------------- :) +(: Numeric family is by VALUE, not by lossy cast: a non-integral lookup :) +(: must not match an integer key (no truncation). :) +(: ----------------------------------------------------------------------- :) + +declare + %test:name("integer key does not match a non-integral double lookup (contains)") + %test:assertFalse +function skt:integer-key-vs-fractional-double-contains() { + map:contains(map { 1: "x" }, 1.5e0) +}; + +declare + %test:name("integer key does not match a non-integral double lookup (get)") + %test:assertEmpty +function skt:integer-key-vs-fractional-double-get() { + map:get(map { 1: "x" }, 1.5e0) +}; + +(: ----------------------------------------------------------------------- :) +(: Within-family positives: members of the SAME op:same-key family that :) +(: are value-equal MUST interchange (these must not regress). :) +(: ----------------------------------------------------------------------- :) + +declare + %test:name("numeric family: integer key matches equal double lookup (contains)") + %test:assertTrue +function skt:numeric-integer-key-vs-double-contains() { + map:contains(map { 5: 1 }, 5.0e0) +}; + +declare + %test:name("numeric family: integer key matches equal double lookup (get)") + %test:assertEquals(1) +function skt:numeric-integer-key-vs-double-get() { + map:get(map { 5: 1 }, 5.0e0) +}; + +declare + %test:name("numeric family: decimal key matches equal integer lookup (contains)") + %test:assertTrue +function skt:numeric-decimal-key-vs-integer-contains() { + map:contains(map { 5.0: 1 }, 5) +}; + +declare + %test:name("string family: string key matches equal xs:anyURI lookup (contains)") + %test:assertTrue +function skt:string-key-vs-anyuri-contains() { + map:contains(map { "urn:x": 1 }, xs:anyURI("urn:x")) +}; + +declare + %test:name("string family: string key matches equal xs:untypedAtomic lookup (get)") + %test:assertEquals("x") +function skt:string-key-vs-untypedatomic-get() { + map:get(map { "12": "x" }, xs:untypedAtomic("12")) +}; + +declare + %test:name("string family: anyURI key matches equal string lookup (contains)") + %test:assertTrue +function skt:anyuri-key-vs-string-contains() { + map:contains(map { xs:anyURI("urn:x"): 1 }, "urn:x") +}; + +(: ----------------------------------------------------------------------- :) +(: Same-type self-matches still work after the fix. :) +(: ----------------------------------------------------------------------- :) + +declare + %test:name("string key matches equal string lookup (get)") + %test:assertEquals("x") +function skt:string-key-self-get() { + map:get(map { "12": "x" }, "12") +}; + +declare + %test:name("xs:date key matches equal xs:date lookup (contains)") + %test:assertTrue +function skt:date-key-self-contains() { + map:contains(map { xs:date("2020-01-01"): 1 }, xs:date("2020-01-01")) +}; diff --git a/exist-core/src/test/xquery/xquery3/fnSerializeNewline.xqm b/exist-core/src/test/xquery/xquery3/fnSerializeNewline.xqm index 74b22dc20b1..a9c34e557aa 100644 --- a/exist-core/src/test/xquery/xquery3/fnSerializeNewline.xqm +++ b/exist-core/src/test/xquery/xquery3/fnSerializeNewline.xqm @@ -49,20 +49,25 @@ function ser:exist-insert-final-newline-true() { declare %test:assertTrue -function ser:exist-insert-final-newline-false-json() { +function ser:exist-insert-final-newline-true-json() { let $doc := map { "a": 1 } let $serialized := fn:serialize($doc, map { "method": "json", - "exist:insert-final-newline": false() + xs:QName("exist:insert-final-newline"): true() } ) - return fn:ends-with($serialized, "}") + return fn:ends-with($serialized, " ") }; +(: An eXist serialization parameter keyed by the prefixed string "exist:..." is + : NON-conformant and is now ignored - only the xs:QName key form is honored, + : because op:same-key treats a string key and a QName key as distinct. Passing + : true() via the string key therefore has no effect, so the default + : (insert-final-newline=false) applies and no trailing newline is added. :) declare %test:assertTrue -function ser:exist-insert-final-newline-true-json() { +function ser:exist-insert-final-newline-json-prefixed-string-ignored() { let $doc := map { "a": 1 } let $serialized := fn:serialize($doc, map { @@ -70,5 +75,5 @@ function ser:exist-insert-final-newline-true-json() { "exist:insert-final-newline": true() } ) - return fn:ends-with($serialized, " ") + return fn:ends-with($serialized, "}") }; diff --git a/exist-core/src/test/xquery/xquery3/serialize.xql b/exist-core/src/test/xquery/xquery3/serialize.xql index 4ac541f0f16..159657f2384 100644 --- a/exist-core/src/test/xquery/xquery3/serialize.xql +++ b/exist-core/src/test/xquery/xquery3/serialize.xql @@ -681,14 +681,16 @@ function ser:exist-output-doctype-QName($value as xs:boolean) { map { xs:QName("exist:output-doctype") : $value }) }; +(: An eXist serialization parameter keyed by the prefixed string "exist:..." is + : NON-conformant and is now ignored - only the xs:QName key form is honored, + : because op:same-key treats a string key and a QName key as distinct. Passing + : true() via the string key therefore has no effect, so the default (no doctype) + : applies. The xs:QName form is exercised by ser:exist-output-doctype-QName above. :) declare - %test:args("true") - %test:assertXPath("contains($result, '-//OASIS//DTD DITA BookMap//EN') and contains($result, 'bookmap.dtd')") - %test:args("false") %test:assertXPath("not(contains($result, '-//OASIS//DTD DITA BookMap//EN')) and not(contains($result, 'bookmap.dtd'))") -function ser:exist-output-doctype-string($value as xs:boolean) { +function ser:exist-output-doctype-prefixed-string-ignored() { serialize(doc($ser:collection || "/test-with-doctype.xml"), - map { "exist:output-doctype" : $value }) + map { "exist:output-doctype" : true() }) }; declare @@ -701,14 +703,14 @@ function ser:exist-expand-xinclude-QName($value as xs:boolean) { map { xs:QName("exist:expand-xincludes"): $value }) }; +(: Prefixed-string key is ignored (see note above); passing false() has no + : effect, so the default (expand-xincludes=true) applies and the include is + : expanded to its 'comment' content. :) declare - %test:args("true") %test:assertXPath("contains($result, 'comment')") - %test:args("false") - %test:assertXPath("contains($result, 'include')") -function ser:exist-expand-xinclude-string($value as xs:boolean) { +function ser:exist-expand-xinclude-prefixed-string-ignored() { serialize($ser:xi-doc, - map { "exist:expand-xincludes": $value }) + map { "exist:expand-xincludes": false() }) }; declare @@ -723,16 +725,13 @@ function ser:exist-add-exist-id-QName($value as xs:string) { map { xs:QName("exist:add-exist-id"): $value }) }; +(: Prefixed-string key is ignored (see note above); passing "all" has no effect, + : so the default (add-exist-id=none) applies and no exist:id attributes appear. :) declare - %test:args("all") - %test:assertEquals('123') - %test:args("element") - %test:assertEquals('123') - %test:args("none") %test:assertXPath("not(contains($result, 'exist:id'))") -function ser:exist-add-exist-id-string($value as xs:string) { +function ser:exist-add-exist-id-prefixed-string-ignored() { serialize(doc($ser:collection || "/test.xml"), - map { "exist:add-exist-id": $value }) + map { "exist:add-exist-id": "all" }) }; declare @@ -750,17 +749,16 @@ function ser:exist-jsonp-QName($value as xs:string) { ) }; +(: Prefixed-string key is ignored (see note above); passing "functionName" has + : no effect, so the default (no JSONP callback) applies and plain JSON results. :) declare - %test:args("functionName") - %test:assertEquals('functionName({"author":["John Doe","Robert Smith"]})') - %test:args("anotherName") - %test:assertEquals('anotherName({"author":["John Doe","Robert Smith"]})') -function ser:exist-jsonp-string($value as xs:string) { + %test:assertEquals('{"author":["John Doe","Robert Smith"]}') +function ser:exist-jsonp-prefixed-string-ignored() { serialize($ser:in-memory-book, map { "method": "json", "media-type": "application/json", - "exist:jsonp": $value + "exist:jsonp": "functionName" } ) }; @@ -775,14 +773,13 @@ function ser:exist-process-xsl-pi-QName($value as xs:boolean) { map { xs:QName("exist:process-xsl-pi"): $value }) }; +(: Prefixed-string key is ignored (see note above); passing false() has no + : effect, so the default (process-xsl-pi=true) applies and the PI is processed. :) declare - %test:args("true") %test:assertEquals('processed') - %test:args("false") - %test:assertXPath("contains($result, 'stylesheet')") -function ser:exist-process-xsl-pi-string($value as xs:boolean) { +function ser:exist-process-xsl-pi-prefixed-string-ignored() { serialize(doc($ser:collection || "/test-xsl.xml"), - map { "exist:process-xsl-pi": $value }) + map { "exist:process-xsl-pi": false() }) }; declare diff --git a/extensions/modules/file/src/test/xquery/modules/file/sync-serialize.xqm b/extensions/modules/file/src/test/xquery/modules/file/sync-serialize.xqm index c26c201a77a..cca0300211e 100644 --- a/extensions/modules/file/src/test/xquery/modules/file/sync-serialize.xqm +++ b/extensions/modules/file/src/test/xquery/modules/file/sync-serialize.xqm @@ -207,7 +207,7 @@ function syse:insert-final-newline-yes() { let $sync := file:sync( $fixtures:collection, $directory, - map{ "exist:insert-final-newline": true() } + map{ xs:QName("exist:insert-final-newline"): true() } ) return ( @@ -231,7 +231,7 @@ function syse:insert-final-newline-no() { let $sync := file:sync( $fixtures:collection, $directory, - map{ "exist:insert-final-newline": false() } + map{ xs:QName("exist:insert-final-newline"): false() } ) return (