From 0dc05bf5859abc4a3fe4e4607f1e83c50f0b1758 Mon Sep 17 00:00:00 2001 From: Michael Ilyin Date: Sat, 18 Jul 2026 00:53:56 +0200 Subject: [PATCH 1/3] Parameters becomes a thin facade over the shared string-backed implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-SDK map-backed parser duplicated zenoh-kotlin's and diverged from Rust: it percent-decoded values, rejected duplicated keys (throwing on attacker-controlled selector parameters in the queryable upcall), and normalized eagerly. io.zenoh.query.Parameters now delegates every operation to the shared io.zenoh.jni.query.Parameters (zenoh-flat-jni), a pure-Kotlin string-backed mirror of Rust's zenoh-protocol parameters.rs — construction is infallible on any input with zero JNI crossings. Behavior changes, all aligning with Rust: no percent-decoding; from(String) never throws; duplicated keys accepted with first-match-wins get (last wins in toMap, as Rust's HashMap conversion); trailing ';'/'='/'|' trimmed at construction; toString round-trips the stored string; equality is string equality. New ParametersCorrespondenceTest validates the shared implementation against the native parameters_get/insert/is_well_formed oracle (zenoh-flat#4) over edge shapes and 500 randomized inputs — it caught the trailing-separator trim rule. remove is correspondence-exempt: upstream zenoh's parameters::remove has an iterator-consumption bug (find advances past entries preceding the first match before filter runs, dropping them; removing an absent key erases everything); the shared implementation follows the documented 'preserving the insertion order' contract instead, and nativeRemoveBugCanary pins the buggy native behavior so the exemption is removed when upstream fixes it. Also folds in the RX hardening that motivated the unification: queryCallbackOf frees the owned native leaves and finalizes the query if decomposition ever throws, and QueryParametersTest sends a=1;a=2;bad=%zz through the raw bindings end to end. CI pins bump to zenoh-flat#4 and zenoh-flat-jni#9. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 4 +- ZENOH_FLAT_TRANSITION.md | 3 +- .../kotlin/io/zenoh/FlatCallbacks.kt | 16 +- .../kotlin/io/zenoh/query/Parameters.kt | 81 +++++----- .../commonMain/kotlin/io/zenoh/query/Query.kt | 3 + .../kotlin/io/zenoh/query/Selector.kt | 14 +- .../io/zenoh/ParametersCorrespondenceTest.kt | 145 ++++++++++++++++++ .../kotlin/io/zenoh/QueryParametersTest.kt | 106 +++++++++++++ 8 files changed, 314 insertions(+), 58 deletions(-) create mode 100644 zenoh-java/src/jvmTest/kotlin/io/zenoh/ParametersCorrespondenceTest.kt create mode 100644 zenoh-java/src/jvmTest/kotlin/io/zenoh/QueryParametersTest.kt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b20aac79..8dc2963d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,14 +31,14 @@ jobs: uses: actions/checkout@v4 with: repository: ZettaScaleLabs/zenoh-flat-jni - ref: dbb1f8c6509e333f49d1c03ecb84dcf07b7af7b8 + ref: 6a19a646e722eb9be844c0d3f5eda9002b2e95be path: zenoh-flat-jni - name: Check out zenoh-flat uses: actions/checkout@v4 with: repository: ZettaScaleLabs/zenoh-flat - ref: 6d22091d75912be1c14330b0f78fa1d4bf258ea3 + ref: 925ea5b2ffd534b3d0632be7d002e6a022262dce path: zenoh-flat - name: Use prebindgen from GitHub main diff --git a/ZENOH_FLAT_TRANSITION.md b/ZENOH_FLAT_TRANSITION.md index f3b79d31..ba9fa709 100644 --- a/ZENOH_FLAT_TRANSITION.md +++ b/ZENOH_FLAT_TRANSITION.md @@ -27,7 +27,8 @@ zenoh (Rust) | --- | --- | --- | | [#481](https://github.com/eclipse-zenoh/zenoh-java/pull/481) | Use receiver-style zenoh-flat-jni bindings (generated Session/Query/Publisher methods, split key-expr overloads, de-prefixed callback names); +61–83% subscriber throughput | merged | | [#484](https://github.com/eclipse-zenoh/zenoh-java/pull/484) | `Encoding`/`ZenohId.toString` become pure JVM values (fixes a per-message native leak); the conversion logic lives in the **shared tier** (`EncodingCodec`/`zidString` in zenoh-flat-jni, correspondence-tested against native, reusable by zenoh-kotlin) with only a thin facade here; publisher **default encoding set natively at declare** (plain puts cross no encoding data). Pairs with [zenoh-flat-jni#4](https://github.com/ZettaScaleLabs/zenoh-flat-jni/pull/4), [zenoh-flat#3](https://github.com/ZettaScaleLabs/zenoh-flat/pull/3) (merged), [prebindgen#80](https://github.com/milyin/prebindgen/pull/80) (merged) | in progress | -| [#491](https://github.com/eclipse-zenoh/zenoh-java/pull/491) | `Session.undeclare` detaches the key-expr handle even when the native undeclare fails (the generated wrapper consumes it either way); found on the zenoh-kotlin port ([zenoh-kotlin#668](https://github.com/eclipse-zenoh/zenoh-kotlin/pull/668)) | open | +| [#491](https://github.com/eclipse-zenoh/zenoh-java/pull/491) | `Session.undeclare` detaches the key-expr handle even when the native undeclare fails (the generated wrapper consumes it either way); found on the zenoh-kotlin port ([zenoh-kotlin#668](https://github.com/eclipse-zenoh/zenoh-kotlin/pull/668)) | merged | +| shared-parameters | `Parameters` becomes a thin facade over the shared string-backed implementation in zenoh-flat-jni (Rust `parameters.rs` semantics: no percent-decoding, infallible parse, first-match-wins get) + `ParametersCorrespondenceTest` vs the native oracle. Pairs with [zenoh-flat#4](https://github.com/ZettaScaleLabs/zenoh-flat/pull/4), [zenoh-flat-jni#9](https://github.com/ZettaScaleLabs/zenoh-flat-jni/pull/9) | open | Companion PRs in the upstream repos are coordinated per constituent PR (e.g. [ZettaScaleLabs/zenoh-flat-jni#3](https://github.com/ZettaScaleLabs/zenoh-flat-jni/pull/3) diff --git a/zenoh-java/src/commonMain/kotlin/io/zenoh/FlatCallbacks.kt b/zenoh-java/src/commonMain/kotlin/io/zenoh/FlatCallbacks.kt index 20dbae09..aec2274a 100644 --- a/zenoh-java/src/commonMain/kotlin/io/zenoh/FlatCallbacks.kt +++ b/zenoh-java/src/commonMain/kotlin/io/zenoh/FlatCallbacks.kt @@ -54,7 +54,21 @@ internal fun queryCallbackOf( // on a channel by a queue handler) and replied to later. The native query // is dropped when it is replied to (see [Query.reply]) or when [Query] is // closed — that drop is what finalizes the querier's get. - f(Query.fromParts(keStr, parameters, payloadH, encId, encSchema, attachH, acceptsReplies, zq)) + val query = try { + Query.fromParts(keStr, parameters, payloadH, encId, encSchema, attachH, acceptsReplies, zq) + } catch (t: Throwable) { + // Defense in depth: the leaves carry remote (attacker-controlled) + // data, and fromParts must be total on it — but if decomposition + // ever throws, free the owned native leaves (payload/attachment + // have no GC backstop) and finalize the query so the remote get + // completes, instead of leaking them. The rethrown exception is + // swallowed by the native callback bridge, by design. + payloadH?.close() + attachH?.close() + zq.close() + throw t + } + f(query) } internal fun replyCallbackOf( diff --git a/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Parameters.kt b/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Parameters.kt index 241ade35..aa27be94 100644 --- a/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Parameters.kt +++ b/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Parameters.kt @@ -14,90 +14,74 @@ package io.zenoh.query -import java.net.URLDecoder +import io.zenoh.jni.query.Parameters as JniParameters /** * Parameters of the [Selector]. * - * When in string form, the `parameters` should be encoded like the query section of a URL: - * - parameters are separated by `;`, + * A thin facade over the shared string-backed implementation + * ([io.zenoh.jni.query.Parameters]), which mirrors Rust's + * `zenoh::query::Parameters` exactly. The string form follows the + * `a=b;c=d|e;f=g` format: + * - parameters are separated by `;` (empty chunks are skipped), * - the parameter name and value are separated by the first `=`, * - in the absence of `=`, the parameter value is considered to be the empty string, - * - both name and value should use percent-encoding to escape characters, - * - defining a value for the same parameter name twice is considered undefined behavior and an - * error result is returned. + * - the value is taken verbatim (no percent-decoding), + * - a duplicated parameter name is allowed: [get] returns the FIRST + * occurrence, and the duplicates survive round-tripping until an + * [insert]/[remove] normalizes the string. * * @see Selector */ -data class Parameters internal constructor(private val params: MutableMap) : IntoParameters { +class Parameters internal constructor(internal val inner: JniParameters) : IntoParameters { companion object { - private const val LIST_SEPARATOR = ";" - private const val FIELD_SEPARATOR = "=" - private const val VALUE_SEPARATOR = "|" - /** * Creates an empty Parameters. */ @JvmStatic - fun empty() = Parameters(mutableMapOf()) + fun empty() = Parameters(JniParameters.empty()) /** * Creates a [Parameters] instance from the provided map. */ @JvmStatic - fun from(params: Map): Parameters = Parameters(params.toMutableMap()) + fun from(params: Map): Parameters = Parameters(JniParameters.fromMap(params)) /** - * Attempts to create a [Parameters] from a string. + * Creates a [Parameters] from a string in the `a=b;c=d|e;f=g` format. * - * When in string form, the `parameters` should be encoded like the query section of a URL: - * - parameters are separated by `;`, - * - the parameter name and value are separated by the first `=`, - * - in the absence of `=`, the parameter value is considered to be the empty string, - * - both name and value should use percent-encoding to escape characters, - * - defining a value for the same parameter name twice is considered undefined behavior and an - * error result is returned. + * Construction is infallible — any string is accepted, exactly as in + * the Rust layer. */ @JvmStatic - fun from(params: String): Parameters { - if (params.isBlank()) { - return Parameters(mutableMapOf()) - } - return params.split(LIST_SEPARATOR).fold(mutableMapOf()) { parametersMap, parameter -> - val (key, value) = parameter.split(FIELD_SEPARATOR).let { it[0] to it.getOrNull(1) } - require(!parametersMap.containsKey(key)) { "Duplicated parameter `$key` detected." } - parametersMap[key] = value?.let { URLDecoder.decode(it, Charsets.UTF_8.name()) } ?: "" - parametersMap - }.let { Parameters(it) } - } + fun from(params: String): Parameters = Parameters(JniParameters.fromString(params)) } - override fun toString(): String = - params.entries.joinToString(LIST_SEPARATOR) { "${it.key}$FIELD_SEPARATOR${it.value}" } + override fun toString(): String = inner.toString() override fun into(): Parameters = this /** * Returns empty if no parameters were provided. */ - fun isEmpty(): Boolean = params.isEmpty() + fun isEmpty(): Boolean = inner.isEmpty() /** * Returns true if the [key] is contained. */ - fun containsKey(key: String): Boolean = params.containsKey(key) + fun containsKey(key: String): Boolean = inner.containsKey(key) /** - * Returns the value of the [key] if present. + * Returns the value of the [key] if present (the first occurrence wins). */ - fun get(key: String): String? = params[key] + fun get(key: String): String? = inner.get(key) /** * Returns the value of the [key] if present, or if not, the [default] value provided. */ - fun getOrDefault(key: String, default: String): String = params.getOrDefault(key, default) + fun getOrDefault(key: String, default: String): String = inner.getOrDefault(key, default) /** * Returns the values of the [key] if present. @@ -108,25 +92,25 @@ data class Parameters internal constructor(private val params: MutableMap? = params[key]?.split(VALUE_SEPARATOR) + fun values(key: String): List? = if (inner.containsKey(key)) inner.values(key) else null /** * Inserts the key-value pair into the parameters, returning the old value * in case of it being already present. */ - fun insert(key: String, value: String): String? = params.put(key, value) + fun insert(key: String, value: String): String? = inner.insert(key, value) /** * Removes the [key] parameter, returning its value. */ - fun remove(key: String): String? = params.remove(key) + fun remove(key: String): String? = inner.remove(key) /** * Extends the parameters with the [parameters] provided, overwriting * any conflicting params. */ fun extend(parameters: IntoParameters) { - params.putAll(parameters.into().params) + inner.extend(parameters.into().inner) } /** @@ -134,13 +118,18 @@ data class Parameters internal constructor(private val params: MutableMap) { - params.putAll(parameters) + inner.extend(parameters) } /** - * Returns a map with the key value pairs of the parameters. + * Returns a map with the key value pairs of the parameters. For a + * duplicated key the last occurrence wins (unlike [get]). */ - fun toMap(): Map = params + fun toMap(): Map = inner.toMap() + + override fun equals(other: Any?): Boolean = other is Parameters && inner == other.inner + + override fun hashCode(): Int = inner.hashCode() } interface IntoParameters { diff --git a/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Query.kt b/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Query.kt index 283aacb4..14132c2e 100644 --- a/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Query.kt +++ b/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Query.kt @@ -73,6 +73,9 @@ class Query internal constructor( zq: io.zenoh.jni.query.Query, ): Query { val ke = KeyExpr(keStr) + // The parameters string is ATTACKER-CONTROLLED (the Rust layer + // forwards any selector parameters untouched) — the shared + // string-backed Parameters accepts any input, never throwing. val selector = if (parameters.isEmpty()) Selector(ke) else Selector(ke, Parameters.from(parameters)) return Query( diff --git a/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Selector.kt b/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Selector.kt index 34e5cd4f..9dd549ca 100644 --- a/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Selector.kt +++ b/zenoh-java/src/commonMain/kotlin/io/zenoh/query/Selector.kt @@ -28,13 +28,12 @@ import io.zenoh.keyexpr.KeyExpr * * When in string form, selectors look a lot like a URI, with similar semantics: * - the `key_expr` before the first `?` must be a valid key expression. - * - the `parameters` after the first `?` should be encoded like the query section of a URL: + * - the `parameters` after the first `?` follow the `a=b;c=d|e;f=g` format: * - parameters are separated by `;`, * - the parameter name and value are separated by the first `=`, * - in the absence of `=`, the parameter value is considered to be the empty string, - * - both name and value should use percent-encoding to escape characters, - * - defining a value for the same parameter name twice is considered undefined behavior, - * with the encouraged behaviour being to reject operations when a duplicate parameter is detected. + * - the value is taken verbatim (no percent-decoding is applied), + * - a duplicated parameter name is allowed; lookups return the first occurrence. * * Zenoh intends to standardize the usage of a set of parameter names. To avoid conflicting with RPC parameters, * the Zenoh team has settled on reserving the set of parameter names that start with non-alphanumeric characters. @@ -60,13 +59,12 @@ data class Selector(val keyExpr: KeyExpr, val parameters: Parameters? = null) : * * When in string form, selectors look a lot like a URI, with similar semantics: * - the `key_expr` before the first `?` must be a valid key expression. - * - the `parameters` after the first `?` should be encoded like the query section of a URL: + * - the `parameters` after the first `?` follow the `a=b;c=d|e;f=g` format: * - parameters are separated by `;`, * - the parameter name and value are separated by the first `=`, * - in the absence of `=`, the parameter value is considered to be the empty string, - * - both name and value should use percent-encoding to escape characters, - * - defining a value for the same parameter name twice is considered undefined behavior, - * with the encouraged behaviour being to reject operations when a duplicate parameter is detected. + * - the value is taken verbatim (no percent-decoding is applied), + * - a duplicated parameter name is allowed; lookups return the first occurrence. * * @param selector The selector expression as a String. * @return An instance [Selector]. diff --git a/zenoh-java/src/jvmTest/kotlin/io/zenoh/ParametersCorrespondenceTest.kt b/zenoh-java/src/jvmTest/kotlin/io/zenoh/ParametersCorrespondenceTest.kt new file mode 100644 index 00000000..8f56c23d --- /dev/null +++ b/zenoh-java/src/jvmTest/kotlin/io/zenoh/ParametersCorrespondenceTest.kt @@ -0,0 +1,145 @@ +// +// Copyright (c) 2026 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// + +package io.zenoh + +import io.zenoh.exceptions.throwZError0 +import io.zenoh.jni.query.Parameters as JniParameters +import io.zenoh.jni.query.parametersGet +import io.zenoh.jni.query.parametersInsert +import io.zenoh.jni.query.parametersIsWellFormed +import io.zenoh.jni.query.parametersRemove +import org.junit.Assert.assertEquals +import org.junit.Test +import kotlin.random.Random + +/** + * Correspondence tests for the shared pure-JVM + * [io.zenoh.jni.query.Parameters] implementation. + * + * The shared tier implements selector-parameters parsing in pure Kotlin (no + * JNI crossing on the production path), on the contract that any JVM-side + * reimplementation of zenoh-flat semantics must be verified against the + * native implementation. These tests drive both — the pure implementation and + * the native oracle (`parametersGet`/`parametersInsert`/`parametersRemove`/ + * `parametersIsWellFormed`, thin wrappers over `zenoh::query::Parameters`) — + * over edge shapes and randomized inputs, asserting equal results. + */ +class ParametersCorrespondenceTest { + + /** Edge shapes of the format rules. */ + private val edgeCases = listOf( + "", + ";", + ";;", + "a", + "a=", + "=v", + "=", + "a=1", + "a=1;b=2", + "a=1;a=2", + "a=b=c", + "a==", + "k=%zz", + "k=%20", + "flag;a=1", + ";;a=1;;b=2;;", + "c=1|2|3", + "c=|", + "c=ified|", + "ключ=значение", + "a=1;=2;b=3", + " a = 1 ; b = 2 ", + "a;a;a", + "a=1;b;a=2", + ) + + private val keys = listOf("a", "b", "c", "k", "flag", "", "missing", "ключ", " a ") + + private fun assertCorrespondence(s: String) { + val pure = JniParameters.fromString(s) + for (k in keys) { + assertEquals( + "get(\"$k\") diverges for input \"$s\"", + parametersGet(s, k, throwZError0), + pure.get(k), + ) + } + assertEquals( + "isWellFormed diverges for input \"$s\"", + parametersIsWellFormed(s, throwZError0), + pure.isWellFormed(), + ) + for (k in keys) { + assertEquals( + "insert(\"$k\", \"val\") diverges for input \"$s\"", + parametersInsert(s, k, "val", throwZError0), + JniParameters.fromString(s).also { it.insert(k, "val") }.asString(), + ) + } + // `remove` is deliberately NOT oracle-compared: see + // [nativeRemoveBugCanary]. + } + + @Test + fun edgeCasesMatchNativeOracle() { + for (s in edgeCases) { + assertCorrespondence(s) + } + } + + @Test + fun randomizedInputsMatchNativeOracle() { + // Random strings over an alphabet dense in separators, so structural + // collisions (duplicate keys, empty chunks, '=' in values) are common. + val alphabet = "ab;=|%; =;" + val rng = Random(20260718) + repeat(500) { + val s = buildString { + repeat(rng.nextInt(0, 24)) { append(alphabet[rng.nextInt(alphabet.length)]) } + } + assertCorrespondence(s) + } + } + + /** The shared implementation follows Rust's DOCUMENTED remove contract + * ("preserving the insertion order"). */ + @Test + fun removeFollowsTheDocumentedContract() { + fun removed(s: String, k: String): String = + JniParameters.fromString(s).also { it.remove(k) }.asString() + + assertEquals("b=2;c=3", removed("b=2;a=1;c=3", "a")) + assertEquals("b=2", removed("a=1;b=2;a=3", "a")) + assertEquals("x=1;y=2", removed("x=1;y=2", "missing")) + assertEquals("", removed("a=1", "a")) + assertEquals("a=1", removed("flag;a=1", "flag")) + assertEquals("1", JniParameters.fromString("b=2;a=1").remove("a")) + } + + /** CANARY: upstream zenoh's `parameters::remove` has an + * iterator-consumption bug — `find` advances past every entry up to and + * including the first match before `filter` builds the result, so + * entries PRECEDING the match are dropped, and removing an absent key + * erases the whole string. The shared implementation intentionally + * diverges (see [removeFollowsTheDocumentedContract]). This test pins + * the buggy native behavior: when upstream fixes it, this test fails — + * then delete it and fold `remove` into [assertCorrespondence]. */ + @Test + fun nativeRemoveBugCanary() { + assertEquals("c=3", parametersRemove("b=2;a=1;c=3", "a", throwZError0)) + assertEquals("", parametersRemove("x=1;y=2", "missing", throwZError0)) + } +} diff --git a/zenoh-java/src/jvmTest/kotlin/io/zenoh/QueryParametersTest.kt b/zenoh-java/src/jvmTest/kotlin/io/zenoh/QueryParametersTest.kt new file mode 100644 index 00000000..ffd3b180 --- /dev/null +++ b/zenoh-java/src/jvmTest/kotlin/io/zenoh/QueryParametersTest.kt @@ -0,0 +1,106 @@ +// +// Copyright (c) 2026 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// + +package io.zenoh + +import io.zenoh.exceptions.throwZError +import io.zenoh.keyexpr.KeyExpr +import io.zenoh.query.Parameters +import io.zenoh.query.Query +import io.zenoh.query.Reply +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The receive path must be as safe against malicious external data as the + * Rust layer: there a selector's parameters are an unvalidated string view — + * any string is valid and forwarded untouched — so [Parameters] accepts + * anything, with Rust semantics. + */ +class QueryParametersTest { + + @Test + fun parsingAcceptsAnyRemoteInputWithRustSemantics() { + // Duplicated parameter name: accepted, the FIRST occurrence wins on get. + assertEquals("1", Parameters.from("a=1;a=2").get("a")) + + // No percent-decoding: the value is kept verbatim. + assertEquals("%zz", Parameters.from("k=%zz").get("k")) + assertEquals("%20", Parameters.from("k=%20").get("k")) + + // Value containing '=': split on the FIRST '=' only. + assertEquals("b=c", Parameters.from("a=b=c").get("a")) + + // Flag without a value, empty chunks, blank input: all accepted. + assertEquals("", Parameters.from("flag").get("flag")) + assertEquals("1", Parameters.from(";;a=1;;").get("a")) + assertTrue(Parameters.from("").isEmpty()) + + // The string round-trips verbatim (duplicates preserved) until an + // insert/remove normalizes it. + val p = Parameters.from("a=1;a=2;flag") + assertEquals("a=1;a=2;flag", p.toString()) + p.insert("a", "3") + assertEquals("flag;a=3", p.toString()) + } + + @Test + fun queryableReceivesMalformedParametersQuery() { + // A remote (non-JVM) client is free to send selector parameters that + // a URL-style parser would reject — the Rust layer forwards any + // string untouched. The queryable must still receive the query and + // be able to reply. + val session = Zenoh.open(Config.loadDefault()) + val keyExpr = KeyExpr.tryFrom("example/testing/malformed/params") + var received: Query? = null + val queryable = session.declareQueryable(keyExpr, callback = { query -> + received = query + query.reply(keyExpr, "ok") + }) + + var reply: Reply? = null + // Bypass the JVM-side Selector validation, as a remote client would. + session.zSession!!.get( + keyExpr.toString(), // s + "a=1;a=2;bad=%zz", // parameters + 1000L, // timeoutMs + null, // target + null, // consolidation + null, // acceptReplies + null, // congestionControl + null, // priority + null, // express + null, // payload + -1, // encodingSel (absent) + null, // encoding00 + null, // encoding01 + null, // encoding1 + null, // attachment + replyCallbackOf { reply = it }, + {}, // onClose + throwZError, + ) + Thread.sleep(1000) + + assertNotNull(received) + assertEquals("1", received!!.parameters?.get("a")) + assertEquals("%zz", received!!.parameters?.get("bad")) + assertNotNull(reply) + + queryable.close() + session.close() + } +} From d4efffe777ff1c9cb17bbc0b526cde9f2e62d48b Mon Sep 17 00:00:00 2001 From: Michael Ilyin Date: Sat, 18 Jul 2026 12:08:38 +0200 Subject: [PATCH 2/3] Extend parameters correspondence to the full API; reframe the adaptation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit zenoh-flat's parameters functions are regular API (values, contains_key and extend now declared too); the JVM runs the same semantics in pure Kotlin because crossing JNI per string operation is expensive — a JNI peculiarity, not a zenoh-flat design choice. Correspondence now covers get/values/ containsKey/insert/extend/isWellFormed over the edge shapes and 500 randomized inputs; comments and test names updated to say 'native implementation' (remove stays exempt behind nativeRemoveBugCanary until eclipse-zenoh/zenoh#2687 lands). CI pins bump to the reworked zenoh-flat and zenoh-flat-jni heads. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 4 +- .../io/zenoh/EncodingCorrespondenceTest.kt | 6 +-- .../io/zenoh/ParametersCorrespondenceTest.kt | 44 ++++++++++++++----- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8dc2963d..ecb5ddee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,14 +31,14 @@ jobs: uses: actions/checkout@v4 with: repository: ZettaScaleLabs/zenoh-flat-jni - ref: 6a19a646e722eb9be844c0d3f5eda9002b2e95be + ref: 757cc6ad7e346f428ffc5e3252ba01ff4d19ba9b path: zenoh-flat-jni - name: Check out zenoh-flat uses: actions/checkout@v4 with: repository: ZettaScaleLabs/zenoh-flat - ref: 925ea5b2ffd534b3d0632be7d002e6a022262dce + ref: 108ebae6e1e900a682013aabbe6a1c14f52d940f path: zenoh-flat - name: Use prebindgen from GitHub main diff --git a/zenoh-java/src/jvmTest/kotlin/io/zenoh/EncodingCorrespondenceTest.kt b/zenoh-java/src/jvmTest/kotlin/io/zenoh/EncodingCorrespondenceTest.kt index dd61a10c..143002f3 100644 --- a/zenoh-java/src/jvmTest/kotlin/io/zenoh/EncodingCorrespondenceTest.kt +++ b/zenoh-java/src/jvmTest/kotlin/io/zenoh/EncodingCorrespondenceTest.kt @@ -26,13 +26,13 @@ import org.junit.Test * The SDK implements the encoding string ↔ `(id, schema)` conversion in pure * Kotlin (no JNI crossing), on the contract that any JVM-side reimplementation * of zenoh-flat API must be verified against the native implementation. These - * tests drive both implementations — the pure one and the native oracle (the + * tests drive both implementations — the pure one and the native one (the * generated `Encoding` handle methods) — over the whole predefined id range * plus the edge shapes of the parse/render rules, asserting equal results. */ class EncodingCorrespondenceTest { - /** Native oracle: the canonical string of `(id, schema)`. */ + /** Native implementation: the canonical string of `(id, schema)`. */ private fun nativeRender(id: Int, schema: String?): String { val h = JniEncoding.newFromId(id, schema, throwZError0) try { @@ -42,7 +42,7 @@ class EncodingCorrespondenceTest { } } - /** Native oracle: parse a string into `(id, schema, canonical string)`. */ + /** Native implementation: parse a string into `(id, schema, canonical string)`. */ private fun nativeParse(s: String): Triple { val h = JniEncoding.newFromString(s, throwZError0) try { diff --git a/zenoh-java/src/jvmTest/kotlin/io/zenoh/ParametersCorrespondenceTest.kt b/zenoh-java/src/jvmTest/kotlin/io/zenoh/ParametersCorrespondenceTest.kt index 8f56c23d..b56bfcf7 100644 --- a/zenoh-java/src/jvmTest/kotlin/io/zenoh/ParametersCorrespondenceTest.kt +++ b/zenoh-java/src/jvmTest/kotlin/io/zenoh/ParametersCorrespondenceTest.kt @@ -16,10 +16,13 @@ package io.zenoh import io.zenoh.exceptions.throwZError0 import io.zenoh.jni.query.Parameters as JniParameters +import io.zenoh.jni.query.parametersContainsKey +import io.zenoh.jni.query.parametersExtend import io.zenoh.jni.query.parametersGet import io.zenoh.jni.query.parametersInsert import io.zenoh.jni.query.parametersIsWellFormed import io.zenoh.jni.query.parametersRemove +import io.zenoh.jni.query.parametersValues import org.junit.Assert.assertEquals import org.junit.Test import kotlin.random.Random @@ -28,13 +31,15 @@ import kotlin.random.Random * Correspondence tests for the shared pure-JVM * [io.zenoh.jni.query.Parameters] implementation. * - * The shared tier implements selector-parameters parsing in pure Kotlin (no - * JNI crossing on the production path), on the contract that any JVM-side - * reimplementation of zenoh-flat semantics must be verified against the - * native implementation. These tests drive both — the pure implementation and - * the native oracle (`parametersGet`/`parametersInsert`/`parametersRemove`/ - * `parametersIsWellFormed`, thin wrappers over `zenoh::query::Parameters`) — - * over edge shapes and randomized inputs, asserting equal results. + * zenoh-flat exposes parameters processing as regular API (the `parameters*` + * functions in `io.zenoh.jni.query`, thin wrappers over + * `zenoh::query::Parameters`); the JVM production path runs the same + * semantics in pure Kotlin instead, because crossing JNI per string + * operation is expensive — a JNI peculiarity, not a zenoh-flat design + * choice. On the contract that any pure reimplementation of native + * semantics must be verified against the native implementation, these tests + * drive both over edge shapes and randomized inputs, asserting equal + * results. */ class ParametersCorrespondenceTest { @@ -76,6 +81,16 @@ class ParametersCorrespondenceTest { parametersGet(s, k, throwZError0), pure.get(k), ) + assertEquals( + "values(\"$k\") diverges for input \"$s\"", + parametersValues(s, k, throwZError0), + pure.values(k), + ) + assertEquals( + "containsKey(\"$k\") diverges for input \"$s\"", + parametersContainsKey(s, k, throwZError0), + pure.containsKey(k), + ) } assertEquals( "isWellFormed diverges for input \"$s\"", @@ -89,19 +104,26 @@ class ParametersCorrespondenceTest { JniParameters.fromString(s).also { it.insert(k, "val") }.asString(), ) } - // `remove` is deliberately NOT oracle-compared: see - // [nativeRemoveBugCanary]. + assertEquals( + "extend diverges for input \"$s\"", + parametersExtend(s, "zk1=zv1;zk2", throwZError0), + JniParameters.fromString(s) + .also { it.extend(JniParameters.fromString("zk1=zv1;zk2")) } + .asString(), + ) + // `remove` is deliberately NOT compared against the native + // implementation: see [nativeRemoveBugCanary]. } @Test - fun edgeCasesMatchNativeOracle() { + fun edgeCasesMatchNative() { for (s in edgeCases) { assertCorrespondence(s) } } @Test - fun randomizedInputsMatchNativeOracle() { + fun randomizedInputsMatchNative() { // Random strings over an alphabet dense in separators, so structural // collisions (duplicate keys, empty chunks, '=' in values) are common. val alphabet = "ab;=|%; =;" From ca51489bf8b1d5bdf94589f591525677ae7e60b0 Mon Sep 17 00:00:00 2001 From: Michael Ilyin Date: Sat, 18 Jul 2026 12:19:17 +0200 Subject: [PATCH 3/3] ci: pin zenoh-flat to merged main (parameters API, zenoh-flat#4) Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ecb5ddee..ef4e60f8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,7 +38,7 @@ jobs: uses: actions/checkout@v4 with: repository: ZettaScaleLabs/zenoh-flat - ref: 108ebae6e1e900a682013aabbe6a1c14f52d940f + ref: 1ece3df5e64db63f980a43e8ae661bdd5ed760c2 path: zenoh-flat - name: Use prebindgen from GitHub main