From 13a23690c09b8d2a89031c703be4b7da38603f36 Mon Sep 17 00:00:00 2001 From: Clemens Portele Date: Tue, 1 Sep 2026 12:54:17 +0200 Subject: [PATCH 1/2] features: reject empty values in a mutation request body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A property element that is present but carries no value states an empty value. The GML decoder dropped it, so an empty value could not be told apart from an absent property anywhere downstream: it silently discarded what the document said, and a request that set a property to nothing looked like one that left it out. It is now decoded as an empty value, the way a JSON member with "" always was. Decoders can also reject such a value outright, for the input formats alike and without a second parse of the content: a new event handler wraps the handler a decoder emits to, next to the read-only one, and sees the values as the decoder resolved them, so an element that carries its value in xlink:href and one that states xsi:nil need no case of their own. A value the document omits, or states as null, does not reach the handler and is unaffected. The GML decoder no longer describes such a rejection as a parse failure — the document parsed, only the value was rejected. --- .../app/FeatureTokenDecoderGmlFromWfs.java | 5 + .../gml/domain/FeatureTokenDecoderGml.java | 20 +- .../FeatureTokenDecoderGmlInputProfile.java | 11 + ...atureTokenDecoderGmlEmptyValuesSpec.groovy | 226 ++++++++++++++++++ .../domain/FeatureTokenDecoderGeoJson.java | 22 +- .../FeatureTokenDecoderEmptyValuesSpec.groovy | 153 ++++++++++++ .../FeatureEventHandlerEmptyValues.java | 111 +++++++++ 7 files changed, 545 insertions(+), 3 deletions(-) create mode 100644 xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlEmptyValuesSpec.groovy create mode 100644 xtraplatform-features-json/src/test/groovy/de/ii/xtraplatform/features/json/app/FeatureTokenDecoderEmptyValuesSpec.groovy create mode 100644 xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/pipeline/FeatureEventHandlerEmptyValues.java diff --git a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/app/FeatureTokenDecoderGmlFromWfs.java b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/app/FeatureTokenDecoderGmlFromWfs.java index f89c877a9..99dfffdf1 100644 --- a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/app/FeatureTokenDecoderGmlFromWfs.java +++ b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/app/FeatureTokenDecoderGmlFromWfs.java @@ -372,6 +372,11 @@ protected boolean advanceParser() { default: // advanceParser(in); } + } catch (IllegalArgumentException e) { + // Already a rejection of the input in its own words — a read-only property, an empty value, + // an unsupported xsi:type. The document parsed; adding "Could not parse GML" would describe + // the wrong problem, so it is passed through unchanged. + throw e; } catch (Exception e) { throw new IllegalArgumentException("Could not parse GML: " + e.getMessage(), e); } diff --git a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGml.java b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGml.java index 810f60eca..93f9c0b3c 100644 --- a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGml.java +++ b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGml.java @@ -18,6 +18,7 @@ import de.ii.xtraplatform.features.domain.SchemaBase.Type; import de.ii.xtraplatform.features.domain.SchemaConstraints; import de.ii.xtraplatform.features.domain.SchemaMapping; +import de.ii.xtraplatform.features.domain.pipeline.FeatureEventHandlerEmptyValues; import de.ii.xtraplatform.features.domain.pipeline.FeatureEventHandlerReadOnly; import de.ii.xtraplatform.features.domain.pipeline.FeatureEventHandlerSimple.ModifiableContext; import de.ii.xtraplatform.features.domain.pipeline.FeatureTokenBufferSimple; @@ -633,7 +634,10 @@ protected void init() { this.context = createContext(); this.downstream = new FeatureTokenBufferSimple<>( - FeatureEventHandlerReadOnly.of(getDownstream(), inputProfile.getReadOnlyProperties()), + FeatureEventHandlerReadOnly.of( + FeatureEventHandlerEmptyValues.of( + getDownstream(), inputProfile.getRejectEmptyValues()), + inputProfile.getReadOnlyProperties()), context); } @@ -702,6 +706,11 @@ private boolean advanceParser() { // ignore: DTD, SPACE, NAMESPACE, NOTATION_DECLARATION, ENTITY_DECLARATION, // PROCESSING_INSTRUCTION, COMMENT, CDATA. ATTRIBUTE is implicit in START_ELEMENT. } + } catch (IllegalArgumentException e) { + // Already a rejection of the input in its own words — a read-only property, an empty value, + // an unsupported xsi:type. The document parsed; adding "Could not parse GML" would describe + // the wrong problem, so it is passed through unchanged. + throw e; } catch (Exception e) { throw new IllegalArgumentException("Could not parse GML: " + e.getMessage(), e); } @@ -1197,9 +1206,18 @@ private void onEndElement() throws XMLStreamException, java.io.IOException { context.setValueType(Type.STRING); downstream.onValue(context); } else if (frame.pendingXlinkHrefFallback != null) { + // The fallback still wins over an empty value, as it did when the value was dropped. context.setValue(frame.pendingXlinkHrefFallback); context.setValueType(Type.STRING); downstream.onValue(context); + } else { + // The property element was there and carried no value — content that is empty or only + // whitespace, the latter never having reached the buffer. That states an empty value, not + // an absent one, the way a JSON member does with "": dropping it here silently discarded + // what the document states and left the empty value invisible to everything downstream. + context.setValue(""); + context.setValueType(Type.STRING); + downstream.onValue(context); } } else if (frame != null && frame.kind == FrameKind.OBJECT_PROPERTY) { // Re-track the OBJECT_PROPERTY's own path before emitting onObjectEnd — nested child diff --git a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlInputProfile.java b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlInputProfile.java index 9bfcbb494..c6b8f1f8f 100644 --- a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlInputProfile.java +++ b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlInputProfile.java @@ -200,6 +200,17 @@ default String getFeatureMemberElementName() { */ Set getReadOnlyProperties(); + /** + * Whether a value that is a string with no characters or with only whitespace makes the document + * invalid. The check applies to the value a decoder resolves for a property, so an element that + * carries its value in {@code xlink:href} and an element that states {@code xsi:nil} need no case + * of their own. + */ + @Value.Default + default boolean getRejectEmptyValues() { + return false; + } + static FeatureTokenDecoderGmlInputProfile empty() { return ImmutableFeatureTokenDecoderGmlInputProfile.builder().build(); } diff --git a/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlEmptyValuesSpec.groovy b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlEmptyValuesSpec.groovy new file mode 100644 index 000000000..7f982cbcb --- /dev/null +++ b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlEmptyValuesSpec.groovy @@ -0,0 +1,226 @@ +/* + * Copyright 2026 interactive instruments GmbH + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package de.ii.xtraplatform.features.gml.domain + +import de.ii.xtraplatform.crs.domain.EpsgCrs +import de.ii.xtraplatform.features.domain.ImmutableFeatureQuery +import de.ii.xtraplatform.features.domain.ImmutableFeatureSchema +import de.ii.xtraplatform.features.domain.ImmutableSchemaMapping +import de.ii.xtraplatform.features.domain.SchemaBase +import de.ii.xtraplatform.streams.app.ReactiveRx +import de.ii.xtraplatform.streams.domain.Reactive +import spock.lang.Shared +import spock.lang.Specification + +import javax.xml.namespace.QName + +/** + * A property element that is present but carries no value states an empty value, the way a JSON + * member does with {@code ""}. The decoder used to drop it, which made the empty value of a scalar + * property invisible; these specs pin the emission and the {@code rejectEmptyValues} check built on + * it, together with the whitespace that separates child elements, which is not a value of its own. + */ +class FeatureTokenDecoderGmlEmptyValuesSpec extends Specification { + + static final String NS = "http://example.com/ns/1.0" + static final Map NAMESPACES = [ + "ex" : NS, + "gml" : "http://www.opengis.net/gml/3.2", + "xlink": "http://www.w3.org/1999/xlink", + "xsi" : "http://www.w3.org/2001/XMLSchema-instance" + ] + + @Shared Reactive reactive + @Shared Reactive.Runner runner + + def setupSpec() { + reactive = new ReactiveRx() + runner = reactive.runner("test-empty-values") + } + + def cleanupSpec() { + runner.close() + } + + private FeatureTokenDecoderGml decoder(boolean rejectEmptyValues) { + def schema = new ImmutableFeatureSchema.Builder() + .name("party") + .sourcePath("/party") + .type(SchemaBase.Type.OBJECT) + .putProperties2("oid", new ImmutableFeatureSchema.Builder() + .sourcePath("objid") + .type(SchemaBase.Type.STRING) + .role(SchemaBase.Role.ID) + .alias("id")) + .putProperties2("givenName", new ImmutableFeatureSchema.Builder() + .sourcePath("given_name") + .type(SchemaBase.Type.STRING) + .alias("givenName")) + .putProperties2("note", new ImmutableFeatureSchema.Builder() + .sourcePath("note") + .type(SchemaBase.Type.STRING) + .alias("note")) + .build() + return new FeatureTokenDecoderGml( + NAMESPACES, + [new QName(NS, "Party")], + schema, + ImmutableFeatureQuery.builder().type(schema.getName()).build(), + Map.of(schema.getName(), new ImmutableSchemaMapping.Builder() + .targetSchema(schema) + .sourcePathTransformer((path, isValue) -> path) + .build()), + EpsgCrs.of(25832), + Optional.empty(), + Optional.empty(), + ImmutableFeatureTokenDecoderGmlInputProfile.builder() + .useAlias(true) + .rejectEmptyValues(rejectEmptyValues) + .build()) + } + + private static String feature(String properties) { + return '' + properties + '' + } + + private List run(String properties, boolean rejectEmptyValues = false) { + return Reactive.Source.inputStream(new ByteArrayInputStream(feature(properties).getBytes('UTF-8'))) + .via(decoder(rejectEmptyValues)) + .to(Reactive.Sink.reduce([], (list, element) -> { list << element; return list })) + .on(runner).run().toCompletableFuture().join() as List + } + + private static List valueOf(List tokens, String property) { + def values = [] + for (int i = 0; i < tokens.size() - 1; i++) { + if (tokens[i] instanceof List && tokens[i] == [property]) { + values << tokens[i + 1] + } + } + return values + } + + def 'a property element with no content states an empty value'() { + when: + def tokens = run('') + + then: + valueOf(tokens, 'givenName') == [''] + } + + def 'a self-closing property element states an empty value'() { + when: + def tokens = run('') + + then: + valueOf(tokens, 'givenName') == [''] + } + + def 'content of only whitespace states an empty value'() { + when: + def tokens = run(' ') + + then: 'whitespace never reaches the character buffer, so it arrives as no content at all' + valueOf(tokens, 'givenName') == [''] + } + + def 'content is unaffected'() { + when: + def tokens = run('Alex') + + then: + valueOf(tokens, 'givenName') == ['Alex'] + } + + def 'the whitespace that separates property elements is not a value'() { + when: + def tokens = run('\n Alex\n a note\n') + + then: 'the indentation between the properties reaches neither of them nor the feature' + valueOf(tokens, 'givenName') == ['Alex'] + valueOf(tokens, 'note') == ['a note'] + } + + def 'an empty value is rejected under rejectEmptyValues'() { + when: + run('', true) + + then: + Throwable e = thrown() + rootCause(e) instanceof IllegalArgumentException + rootCause(e).message.contains("'givenName' has an empty value") + } + + def 'the rejection is not dressed up as a parse failure'() { + when: + run('', true) + + then: 'the document parsed; only the value was rejected, so the message says only that' + Throwable e = thrown() + def reported = e.cause ?: e + reported.message.startsWith("The property 'givenName' has an empty value") + !reported.message.contains('Could not parse GML') + } + + def 'a self-closing property element is rejected under rejectEmptyValues'() { + when: + run('', true) + + then: + Throwable e = thrown() + rootCause(e) instanceof IllegalArgumentException + } + + def 'content of only whitespace is rejected under rejectEmptyValues'() { + when: + run(' ', true) + + then: + Throwable e = thrown() + rootCause(e) instanceof IllegalArgumentException + } + + def 'a property with content passes under rejectEmptyValues'() { + when: + def tokens = run('Alex', true) + + then: + notThrown(Throwable) + valueOf(tokens, 'givenName') == ['Alex'] + } + + def 'a reference carries its value in xlink:href and is not empty'() { + when: + def tokens = run('', true) + + then: + notThrown(Throwable) + valueOf(tokens, 'note') == ['https://example.com/notes/1'] + } + + def 'xsi:nil states the absence of a value, not an empty one'() { + when: + def tokens = run('', true) + + then: 'no value is decoded at all, so there is nothing to reject' + notThrown(Throwable) + valueOf(tokens, 'note') == [] + } + + private static Throwable rootCause(Throwable e) { + Throwable cause = e + while (cause.getCause() != null && cause.getCause() != cause) { + cause = cause.getCause() + } + cause + } +} diff --git a/xtraplatform-features-json/src/main/java/de/ii/xtraplatform/features/json/domain/FeatureTokenDecoderGeoJson.java b/xtraplatform-features-json/src/main/java/de/ii/xtraplatform/features/json/domain/FeatureTokenDecoderGeoJson.java index 393b4a34d..16fdad75e 100644 --- a/xtraplatform-features-json/src/main/java/de/ii/xtraplatform/features/json/domain/FeatureTokenDecoderGeoJson.java +++ b/xtraplatform-features-json/src/main/java/de/ii/xtraplatform/features/json/domain/FeatureTokenDecoderGeoJson.java @@ -18,6 +18,7 @@ import de.ii.xtraplatform.features.domain.FeatureSchema; import de.ii.xtraplatform.features.domain.SchemaBase.Type; import de.ii.xtraplatform.features.domain.SchemaMapping; +import de.ii.xtraplatform.features.domain.pipeline.FeatureEventHandlerEmptyValues; import de.ii.xtraplatform.features.domain.pipeline.FeatureEventHandlerReadOnly; import de.ii.xtraplatform.features.domain.pipeline.FeatureEventHandlerSimple.ModifiableContext; import de.ii.xtraplatform.features.domain.pipeline.FeatureTokenBufferSimple; @@ -49,6 +50,7 @@ public class FeatureTokenDecoderGeoJson private final Axes axes; private final GeometryDecoderJson geometryDecoder; private final Set readOnlyProperties; + private final boolean rejectEmptyValues; private boolean started; private int depth = -1; @@ -81,18 +83,31 @@ public FeatureTokenDecoderGeoJson( this(nullValue, crs, axes, supportedCrs, Set.of()); } + public FeatureTokenDecoderGeoJson( + Optional nullValue, + EpsgCrs crs, + Axes axes, + List supportedCrs, + Set readOnlyProperties) { + this(nullValue, crs, axes, supportedCrs, readOnlyProperties, false); + } + /** * @param supportedCrs the coordinate reference systems that a {@code coordRefSys} member in the * document may declare; an empty list does not restrict them + * @param rejectEmptyValues whether a value that is a string with no characters or with only + * whitespace makes the document invalid */ public FeatureTokenDecoderGeoJson( Optional nullValue, EpsgCrs crs, Axes axes, List supportedCrs, - Set readOnlyProperties) { + Set readOnlyProperties, + boolean rejectEmptyValues) { super(); this.readOnlyProperties = readOnlyProperties; + this.rejectEmptyValues = rejectEmptyValues; try { this.parser = JSON_FACTORY.createNonBlockingByteArrayParser(); } catch (IOException e) { @@ -110,7 +125,10 @@ protected void init() { this.context = createContext(); this.downstream = new FeatureTokenBufferSimple<>( - FeatureEventHandlerReadOnly.of(getDownstream(), readOnlyProperties), context); + FeatureEventHandlerReadOnly.of( + FeatureEventHandlerEmptyValues.of(getDownstream(), rejectEmptyValues), + readOnlyProperties), + context); } @Override diff --git a/xtraplatform-features-json/src/test/groovy/de/ii/xtraplatform/features/json/app/FeatureTokenDecoderEmptyValuesSpec.groovy b/xtraplatform-features-json/src/test/groovy/de/ii/xtraplatform/features/json/app/FeatureTokenDecoderEmptyValuesSpec.groovy new file mode 100644 index 000000000..d965f7e1b --- /dev/null +++ b/xtraplatform-features-json/src/test/groovy/de/ii/xtraplatform/features/json/app/FeatureTokenDecoderEmptyValuesSpec.groovy @@ -0,0 +1,153 @@ +/* + * Copyright 2026 interactive instruments GmbH + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package de.ii.xtraplatform.features.json.app + +import de.ii.xtraplatform.crs.domain.OgcCrs +import de.ii.xtraplatform.features.json.domain.FeatureTokenDecoderGeoJson +import de.ii.xtraplatform.geometries.domain.Axes +import de.ii.xtraplatform.streams.app.ReactiveRx +import de.ii.xtraplatform.streams.domain.Reactive +import spock.lang.Shared +import spock.lang.Specification + +/** + * The empty-value check rides the decoding of the request body, so these specs drive the real + * decoder rather than a handler in isolation: what reaches the check is what the decoder resolved. + */ +class FeatureTokenDecoderEmptyValuesSpec extends Specification { + + @Shared + Reactive reactive + @Shared + Reactive.Runner runner + + def setupSpec() { + reactive = new ReactiveRx() + runner = reactive.runner("test") + } + + def cleanupSpec() { + runner.close() + } + + private static String feature(String properties, String id = '"B.1"') { + return """ + { + "type": "Feature", + "id": ${id}, + "geometry": {"type": "Point", "coordinates": [8.7, 49.4]}, + "properties": ${properties} + } + """ + } + + def 'an empty string is rejected and the message names the property'() { + when: + run(source(feature('{"function": ""}'), true)) + + then: + Throwable e = thrown() + rootCause(e) instanceof IllegalArgumentException + rootCause(e).message.contains("'function' has an empty value") + } + + def 'a string of only whitespace is rejected'() { + when: + run(source(feature('{"function": " \\t "}'), true)) + + then: + Throwable e = thrown() + rootCause(e) instanceof IllegalArgumentException + } + + def 'an empty value in a nested object is rejected'() { + when: + run(source(feature('{"lifetime": {"end": ""}}'), true)) + + then: + Throwable e = thrown() + rootCause(e).message.contains('lifetime.end') + } + + def 'an empty value in an array is rejected'() { + when: + run(source(feature('{"tags": ["a", "", "c"]}'), true)) + + then: + Throwable e = thrown() + rootCause(e) instanceof IllegalArgumentException + } + + def 'an empty id is rejected'() { + when: + run(source(feature('{"function": "commercial"}', '""'), true)) + + then: + Throwable e = thrown() + rootCause(e) instanceof IllegalArgumentException + } + + def 'a body without empty values is decoded'() { + when: + List tokens = run(source(feature('{"function": "commercial", "count": 0}'), true)) + + then: + notThrown(Throwable) + tokens.contains('commercial') + } + + def 'null states the absence of a value and is not rejected'() { + when: + List tokens = run(source(feature('{"function": null}'), true)) + + then: 'the value never reaches the check, so the feature is decoded' + notThrown(Throwable) + tokens.contains('B.1') + } + + def 'a value that is not a string cannot be empty'() { + when: + List tokens = run(source(feature('{"count": 0, "flag": false}'), true)) + + then: + notThrown(Throwable) + tokens.contains('0') + } + + def 'an empty value is decoded as usual without the option'() { + when: + List tokens = run(source(feature('{"function": ""}'), false)) + + then: + notThrown(Throwable) + tokens.contains('') + } + + private Reactive.Stream> source(String body, boolean rejectEmptyValues) { + Reactive.Source.inputStream(new ByteArrayInputStream(body.getBytes('UTF-8'))) + .via(new FeatureTokenDecoderGeoJson( + Optional.empty(), OgcCrs.CRS84, Axes.XY, List.of(), Set.of(), + rejectEmptyValues)) + .to(Reactive.Sink.reduce([], (list, element) -> { + list << element + return list + })) + } + + private List run(Reactive.Stream> stream) { + stream.on(runner).run().toCompletableFuture().join() + } + + private static Throwable rootCause(Throwable e) { + Throwable cause = e + while (cause.getCause() != null && cause.getCause() != cause) { + cause = cause.getCause() + } + cause + } +} diff --git a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/pipeline/FeatureEventHandlerEmptyValues.java b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/pipeline/FeatureEventHandlerEmptyValues.java new file mode 100644 index 000000000..ef5a70244 --- /dev/null +++ b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/pipeline/FeatureEventHandlerEmptyValues.java @@ -0,0 +1,111 @@ +/* + * Copyright 2026 interactive instruments GmbH + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package de.ii.xtraplatform.features.domain.pipeline; + +import de.ii.xtraplatform.features.domain.pipeline.FeatureEventHandlerSimple.ModifiableContext; + +/** + * Rejects a decoded feature that sets an empty value — a string with no characters or with only + * whitespace — instead of storing it as though the client had supplied something. + * + *

A decoder wraps the handler it emits to, so the check rides the decoding of the request body: + * it sees the values of every format in one place, without a second parse of the content, and it + * sees them as the decoder resolved them, so a value carried by an attribute rather than by element + * content needs no special case of its own. + * + *

Only a value event can carry an empty value. A value that the document omits, or states as + * null, does not reach the handler at all, so the absence of a value is unaffected — and only a + * string can be empty, so where the request body is also validated against a schema this check adds + * exactly what the schema cannot express. + */ +public class FeatureEventHandlerEmptyValues> + implements FeatureEventHandlerSimple { + + private final FeatureEventHandlerSimple delegate; + + private FeatureEventHandlerEmptyValues(FeatureEventHandlerSimple delegate) { + this.delegate = delegate; + } + + /** The handler itself, unless empty values are rejected. */ + public static > FeatureEventHandlerSimple of( + FeatureEventHandlerSimple delegate, boolean rejectEmptyValues) { + return rejectEmptyValues ? new FeatureEventHandlerEmptyValues<>(delegate) : delegate; + } + + /** + * Whether the value is empty: a string with no characters or with only whitespace. {@code null} + * is the absence of a value, not an empty one, and is therefore not empty. + */ + public static boolean isEmpty(String value) { + return value != null && value.isBlank(); + } + + /** The rejection of an empty value, so every call site reports one the same way. */ + public static IllegalArgumentException rejected(String path) { + return new IllegalArgumentException( + String.format( + "The property '%s' has an empty value, a request that changes a feature must not set" + + " one. Omit the property, or state it as null, to leave it without a value.", + path)); + } + + @Override + public void onValue(V context) { + if (isEmpty(context.value())) { + throw rejected(context.pathAsString()); + } + + delegate.onValue(context); + } + + @Override + public void onStart(V context) { + delegate.onStart(context); + } + + @Override + public void onEnd(V context) { + delegate.onEnd(context); + } + + @Override + public void onFeatureStart(V context) { + delegate.onFeatureStart(context); + } + + @Override + public void onFeatureEnd(V context) { + delegate.onFeatureEnd(context); + } + + @Override + public void onObjectStart(V context) { + delegate.onObjectStart(context); + } + + @Override + public void onObjectEnd(V context) { + delegate.onObjectEnd(context); + } + + @Override + public void onArrayStart(V context) { + delegate.onArrayStart(context); + } + + @Override + public void onArrayEnd(V context) { + delegate.onArrayEnd(context); + } + + @Override + public void onGeometry(V context) { + delegate.onGeometry(context); + } +} From 4b04b17f3e73d71dcd94ae04f5dc5881bf4f849e Mon Sep 17 00:00:00 2001 From: Clemens Portele Date: Tue, 1 Sep 2026 12:55:51 +0200 Subject: [PATCH 2/2] fix the exception class name in a feature mutation error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stream that decodes and encodes a mutation is run with join(), which wraps a failure in a CompletionException whose message is the cause's toString() — so every error reported for a failed action was prefixed with the class name of the exception behind it. How the stream is run is nobody else's business, so the wrapper no longer escapes the method that created it. A cause that is itself wrapped now also reaches the encoder-error translation, which turns a database or JSON parse failure into a message written for the client instead of relaying the raw one. --- .../features/sql/app/SqlMutationSession.java | 32 +++++++++++++--- .../sql/app/SqlMutationSessionSpec.groovy | 38 +++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java index f1ae9cf0f..6b897fa78 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java @@ -44,6 +44,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.CompletionException; import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -1664,12 +1665,31 @@ private void drainSource( (a, b) -> a.getRows().isEmpty() ? b : a.patchWith(b))); } - featureSqlSource - .to(Sink.foreach(collected::add)) - .on(streamRunner) - .run() - .toCompletableFuture() - .join(); + try { + featureSqlSource + .to(Sink.foreach(collected::add)) + .on(streamRunner) + .run() + .toCompletableFuture() + .join(); + } catch (CompletionException e) { + // How the stream is run is nobody else's business, so the wrapper join() adds must not + // escape: `Throwable(cause)` takes the wrapper's message from the cause's toString(), which + // puts the exception class name into the message a client eventually reads. + throw unwrap(e); + } + } + + // Package-private so a spec can exercise the contract without the mutation fixture. + static RuntimeException unwrap(CompletionException wrapper) { + Throwable cause = wrapper.getCause(); + if (cause instanceof RuntimeException runtime) { + return runtime; + } + if (cause == null) { + return wrapper; + } + return new IllegalStateException(cause.getMessage(), cause); } // Original per-feature loop, kept for the UPDATE/REPLACE path (each feature is preceded by a diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlMutationSessionSpec.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlMutationSessionSpec.groovy index 7a3ef361f..ec751544b 100644 --- a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlMutationSessionSpec.groovy +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlMutationSessionSpec.groovy @@ -41,6 +41,44 @@ class SqlMutationSessionSpec extends Specification { return new SqlMutationSession(sqlSession, mappings, null, null, null, Optional.empty(), null, Optional.empty()) } + def 'unwrap returns the cause of the join() wrapper, so its class name stays out of the message'() { + given: 'the wrapper CompletableFuture.join() throws — its message is the cause toString()' + def cause = new IllegalArgumentException("The property 'vna' has an empty value.") + def wrapper = new java.util.concurrent.CompletionException(cause) + + expect: 'the wrapper alone would put the class name in front of the message' + wrapper.message.startsWith('java.lang.IllegalArgumentException') + + when: + def unwrapped = SqlMutationSession.unwrap(wrapper) + + then: 'the cause is handed on as-is, so the message a client reads is just the message' + unwrapped.is(cause) + unwrapped.message == "The property 'vna' has an empty value." + } + + def 'unwrap keeps a checked cause reportable without the class-name prefix'() { + given: + def cause = new java.io.IOException('connection reset') + def wrapper = new java.util.concurrent.CompletionException(cause) + + when: + def unwrapped = SqlMutationSession.unwrap(wrapper) + + then: + unwrapped instanceof IllegalStateException + unwrapped.message == 'connection reset' + unwrapped.cause.is(cause) + } + + def 'unwrap falls back to the wrapper when it has no cause'() { + given: + def wrapper = new java.util.concurrent.CompletionException('no cause', null) + + expect: + SqlMutationSession.unwrap(wrapper).is(wrapper) + } + def 'commit delegates to the underlying SqlSession'() { given: def session = buildSession()