buffer-api: expose buffer contents as byte arrays by giaphoang · Pull Request #3696 · apple/servicetalk

giaphoang · 2026-10-24T22:26:11Z

Motivation

Getting a buffer's bytes currently requires callers to account for readable
offsets and the underlying storage type. A discoverable API on Buffer
centralizes this conversion without requiring an internal utility dependency.

Modifications

Add readable-region and indexed Buffer.toByteArray overloads. They copy by
default, preserve reader and writer indices, and validate indexed regions
against the buffer capacity.
Add an explicit optional-copy mode that may reuse an exactly matching backing
array and returns a shared array for all zero-length results.
Cover empty, heap, direct, sliced, read-only, composite, indexed, bounds, and
shared-array cases.
Migrate ByteArraySerializer, Jackson deserializers, and Jersey while
preserving each caller's existing consumption and zero-copy behavior.
Remove the internal BufferUtils, its tests, and its unnecessary
dependencies. Restore the unrelated varint test to the PR base version.

Result

Applications and internal callers can obtain byte arrays through a consistent
public Buffer API, with copying as the safe default and explicit backing-array
reuse when requested.

Validation: 1,027 tests passed across the ten affected modules; nine tests were
skipped. Repository compilation, affected-module quality checks, dependency
analysis, documentation validation, and ./gradlew quality passed.

Closes #863.

#### Motivation Serializers and Jersey request streams duplicate Buffer-to-byte-array conversion. Each copy must handle readable regions and backing-array reuse, making buffer edge cases harder to keep consistent. #### Modifications - Consolidate conversion while preserving each caller's reader-index behavior and the serializer's forced-copy option. - Retain Jackson's offset-aware backing-array path to avoid additional heap-buffer copies. #### Result Empty, heap, direct, and sliced buffers share conversion handling. Validation: 690 tests passed across the eight affected modules; nine tests were skipped. Affected-module quality checks and `./gradlew quality` passed, including dependency analysis and documentation validation. Closes apple#863.

bryce-anderson

Thank you! This is looking pretty good. I just have a few minor suggestions.

idelpivnitskiy · 2026-10-28T04:59:23Z

+     * @param forceCopy whether to always copy into a new array
+     * @return an array containing exactly the readable bytes
+     */
+    public static byte[] toByteArray(final Buffer buffer, final boolean forceCopy) {


@bryce-anderson WDYT if we promote this to Buffer.toByteArray() and Buffer.toByteArray(int, int) API instead of an internal utility? It will be similar to Buffer.toNioBuffer() and Buffer.toString(Charset) API, and will have to follow semantics of not moving indexes.

I've seen this as quite often need to get a byte-array without worrying about offsets. This is why we created ByteArraySerializer, but its discoverability is questionable. A method on Buffer would be much easier for users.

The only trick is that in many other java use-cases it always makes a copy: ByteArrayOutputStream.toByteArray(), ByteString.toByteArray() (protobuf), BigInteger.toByteArray(), etc. So, the default behavior will need to be forceCopy == true and we may have an overload where it skips copying.

Other benefits would be:

We can avoid class name clash with io.servicetalk.buffer.netty.BufferUtils.

servicetalk-buffer-api won't become an API dependency on this module.

I'd buy that.

Thank you for your notice! This actually removes the circular dependency.

Implemented this proposal in the public API commit: Buffer now has readable-region and indexed toByteArray overloads, copies by default, offers explicit optional backing-array reuse, validates bounds, and never moves indices. The caller migration commit removes the internal helper and the utility-module dependencies.

…internal/BufferUtils.java Co-authored-by: Bryce Anderson <bl_anderson@apple.com>

#### Motivation Applications need byte arrays from buffers without duplicating offset calculations or depending on the buffer's storage type. An API on Buffer makes this operation easier to discover than an internal conversion utility. #### Modifications - Add readable-region and indexed toByteArray methods that copy by default and preserve reader and writer indices. - Allow callers to reuse an exactly matching backing array explicitly, with shared empty arrays for all zero-length results. #### Result Callers can obtain byte arrays from heap, direct, sliced, and read-only buffers through the same public API.

Motivation: Callers should use the public Buffer API instead of a duplicated internal conversion helper. Modifications: - Migrate byte-array serialization, Jackson, and Jersey callers. - Preserve each caller's existing consumption and array-sharing behavior. - Remove BufferUtils, its tests, dependencies, and unrelated varint changes. Result: Buffer conversion is centralized without introducing utility-module dependencies.

idelpivnitskiy

Looks great! A few more comments for the new approach:

idelpivnitskiy

Looks great! Thank you!

2 minor nits that I will apply via UI:

idelpivnitskiy · 2026-11-14T00:51:27Z

+
+        assertThat(stringStreamingSerializer(typeRef).deserialize(from(EMPTY_BUFFER, buffer, EMPTY_BUFFER), alloc)
+                .toIterable(), contains("value"));
+        assertThat(buffer.readerIndex(), is(buffer.hasArray() ? 1 : 8));


FYI, these assertions capture existing behavior that violates the Deserializer contract (heap buffers keep readerIndex, direct are consumed). Fine to keep the test as-is, it will be addressed separately in #3731 after this PR is merged.

Thanks for pointing that out and for merging! I'll follow along with #3731

idelpivnitskiy · 2026-11-14T00:51:27Z

+        try (StreamingDeserializer<String> deserializer = serializationProvider.getDeserializer(String.class)) {
+            assertThat(deserializer.deserialize(buffer), contains("value"));
+        }
+        assertThat(buffer.readerIndex(), is(direct ? 8 : 1));


Same here, tracked in #3731.

…/BufferUtils.java

bryce-anderson reviewed Oct 27, 2026

View reviewed changes

bryce-anderson self-assigned this Oct 27, 2026

idelpivnitskiy reviewed Oct 28, 2026

View reviewed changes

giaphoang and others added 3 commits October 29, 2026 19:31

Update servicetalk-utils-internal/src/main/java/io/servicetalk/utils/…

f018ba5

…internal/BufferUtils.java Co-authored-by: Bryce Anderson <bl_anderson@apple.com>

giaphoang changed the title ~~utils-internal: consolidate Buffer to byte[] conversion~~ buffer-api: expose buffer contents as byte arrays Oct 30, 2026

idelpivnitskiy reviewed Oct 30, 2026

View reviewed changes

buffer-api: simplify byte-array conversion and align caller consumption

f00cb95

idelpivnitskiy mentioned this pull request Nov 14, 2026

data-jackson: streaming deserializers don't advance readerIndex for heap buffers #3731

Open

idelpivnitskiy approved these changes Nov 14, 2026

View reviewed changes

idelpivnitskiy added 2 commits November 13, 2026 18:20

Update servicetalk-buffer-api/src/main/java/io/servicetalk/buffer/api…

1744e48

…/BufferUtils.java

Update servicetalk-buffer-api/build.gradle

d51f5bb

idelpivnitskiy requested a review from bryce-anderson November 14, 2026 02:37

Conversation

giaphoang commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Result

Uh oh!

bryce-anderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

idelpivnitskiy Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

bryce-anderson Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

giaphoang Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

giaphoang Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

idelpivnitskiy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

idelpivnitskiy left a comment

Choose a reason for hiding this comment

Uh oh!

idelpivnitskiy Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

giaphoang Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

idelpivnitskiy Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

giaphoang commented Oct 24, 2026 •

edited

Loading