[bugfix] xmldb:store: parse binary content stored under an XML mime type

[joewiz]

[This PR was co-authored with Claude Code. -Joe]

Summary

xmldb:store($collection, $name, $content, $mime) threw a NullPointerException — Cannot invoke "String.length()" because "<parameter1>" is null — whenever $content was an xs:base64Binary value and the target mime type was an XML type (an explicit mime="application/xml", or one inferred from an .xml resource name). The store failed for every binary-content + XML-mime combination, regardless of how the binary value was produced.

Minimal repro:

xmldb:store("/db", "x.xml", xs:base64Binary("PHByb2JlPmhpPC9wcm9iZT4="), "application/xml")
(: -> NullPointerException: Cannot invoke "String.length()" because "<parameter1>" is null :)

Root cause

getResource(mimeType, …) creates an XMLResource for an XML mime type. The binary was then bound to it with resource.setContent((BinaryValue) item). A BinaryValue is an AtomicValue, so LocalXMLResource.setContent routed it into the resource's value slot, leaving the XML resource with no character or byte stream. The store path then built the parse source as Objects.requireNonNullElseGet(res.inputSource, () -> new StringInputSource(res.content)) (LocalCollection.storeXMLResource); with both inputSource and content null, this produced new StringInputSource(null), and parsing failed at new StringReader(null) → the NPE.

What changed

XMLDBStore now branches on the mime type for binary content:

• Binary content + XML mime type → read the bytes and set them on the resource as a re-readable StringInputSource(byte[]), so the parser reads the bytes and detects the encoding from the XML declaration (the bytes are parsed and stored as an XML document). StringInputSource is used because the store may open the source stream more than once — a plain InputSource over a single ByteArrayInputStream would be drained on the first read.
• Binary content + non-XML mime type → unchanged; the BinaryValue is still streamed straight to a binary resource without materializing it.

Malformed bytes under an XML mime now produce a clean parse error instead of an NPE. Reading the bytes into a buffer for the XML case adds no asymptotic memory cost, since storing XML parses the whole document into a DOM anyway.

How it surfaced

This was found via existdb-openapi's binary-safe PUT /api/db/resource. roaster hands the raw request body straight through, and request:get-data() returns it as an xs:base64Binary for an application/octet-stream upload; the handler then calls xmldb:store with the resource's natural (XML) mime, hitting the untested binary-content + XML-mime pairing. request:get-data() itself was never at fault — it delivers the octet-stream body correctly through the controller-forward path; the bug is entirely in xmldb:store.

Test plan

• New XQSuite coverage (exist-core/src/test/xquery/xmldb/store-binary-tests.xql, run by XMLDBTests): binary stored under an explicit application/xml mime and under an inferred .xml mime is parsed and re-readable as XML; the stored doc is a real XML document (not a binary resource); the XML encoding declaration in the bytes is honored; binary + a binary mime still stores byte-for-byte (control); malformed bytes under an XML mime produce a clean store/parse error, not an NPE.
• XMLDBTests green (36/36, including the 30 pre-existing xmldb tests).
• Codacy PMD clean on the changed file.
• Verified the NPE before the fix and its absence after, via a direct xmldb:store call on a running instance.

[reinhapa]

Directly set a input source using the getInputStream() from BinaryValue as all returned implementation will support mark() in case a partial re-read is needed.

Suggested change

	// The content is binary but the target mime type is an XML type: parse the
	// binary's bytes as an XML document. Setting the BinaryValue directly would
	// leave the (XML) resource with no character/byte stream, and the store would
	// later fail with an NPE when it tried to parse a null string as XML. Feed the
	// bytes through an InputSource so the parser reads (and encoding-detects) them;
	// storing XML parses the whole document into a DOM anyway, so reading the bytes
	// into a buffer here adds no asymptotic memory cost over the parse itself.
	final byte[] xmlBytes;
	try (final ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
	((BinaryValue) item).streamBinaryTo(baos);
	xmlBytes = baos.toByteArray();
	} catch (final IOException e) {
	throw new XPathException(this, "Unable to read binary content to store as XML: " + e.getMessage(), e);
	}
	// StringInputSource(byte[]) is re-readable (the store may open the
	// stream more than once) and lets the parser detect the encoding from
	// the bytes; a plain InputSource over a single ByteArrayInputStream would
	// be drained on the first read.
	resource.setContent(new StringInputSource(xmlBytes));
	BinaryValue binaryValue = (BinaryValue)item;
	resource.setContent(new InputSource(binaryValue.getInputStream()));

Also let BinaryValueFromInputStream return a InputStream.nullInputStream() instead of null in the failure case

[joewiz]

[This response was prompted by Joe, drafted by Claude Code, and reviewed by Joe.]

Thanks — I've made the getInputStream() change: BinaryValueFromInputStream.getInputStream() now returns InputStream.nullInputStream() instead of null on its internal-failure path.

On setting the input source directly from getInputStream(): I tried it, and it doesn't survive the xmldb:store path. The store parses the source twice — once to validate, once to store (MutableCollection.storeXmlDocument → validator pass, then store pass) — and it does not reset() the source between the two passes; each pass just re-invokes getByteStream() and parses. A plain InputSource wrapping a single getInputStream() is therefore drained by the validate pass, and the store pass sees an empty stream (fatal error … Premature end of file). So the mark()/reset() support isn't exercised on this route — what the route needs is a source that hands back a fresh stream on each read.

I settled on buffering into a re-readable StringInputSource(byte[]), whose getByteStream() returns a new UnsynchronizedByteArrayInputStream per call. For the XML branch specifically the buffer is asymptotically free — storing XML parses the whole document into a DOM regardless — so this doesn't reintroduce the OOM risk the non-XML branch avoids by streaming. Verified against the four XQSuite cases (explicit and inferred XML mime, encoding honored, stored-as-XML-not-binary) plus the malformed-bytes case; all green.

[dizzzz]

@joewiz please could you revise?

[dizzzz]

Better to use apache's BAOS, or better we use frequenlty a variant that does not do locking/blocking/...

[joewiz]

[This response was prompted by Joe, drafted by Claude Code, and reviewed by Joe.]

Good call — switched to org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream for collecting the bytes, so the write side takes no lock. The read side was already lock-free: StringInputSource hands back an UnsynchronizedByteArrayInputStream. The JDK java.io.ByteArrayOutputStream import is gone.

[joewiz]

@reinhapa Ack, sorry for missing your review!

@dizzzz Thanks for bringing this to my attention!