WebClient corrupts binary data when trying to upload many files

[karolkrasnowski]

I use WebClient to upload binary files. When there are only a few of them everything works great, but when there are more (e.g. several hundred), WebClient sometimes corrupts binary data.
I described the problem in more detail in this question on StackOverflow. I also created a repository containing tests that illustrate this issue.
I don't know what is the reason for this behavior but it doesn't seem entirely deterministic.

My use case is that I am sending binary files from one service (let's call it A) to another (let's call it B).

This is how I read these binary files in the service B:

@RestController
class FilesController {

    @PostMapping(value = "/files")
    Mono<List<String>> uploadFiles(@RequestBody Flux<Part> parts) {
        return parts
                .filter(FilePart.class::isInstance)
                .map(FilePart.class::cast)
                .flatMap(part -> DataBufferUtils.join(part.content())
                        .map(buffer -> {
                            byte[] data = new byte[buffer.readableByteCount()];
                            buffer.read(data);
                            DataBufferUtils.release(buffer);
                            return Base64.getEncoder().encodeToString(data);
                        })
                )
                .collectList();
    }
}

And this is how I send these files in the service A (I wrote some tests to better illustrate the issue).

public class BinaryUploadTest {

    private final List<String> sentBytes = new CopyOnWriteArrayList<>();

    @BeforeEach
    void before() {
        sentBytes.clear();
    }

    /**
     * this test passes all the time
     */
    @Test
    void shouldUpload5Files() {
        // given
        MultiValueMap<String, HttpEntity<?>> body = buildResources(5);

        // when
        List<String> receivedBytes = sendPostRequest(body);

        // then
        assertEquals(sentBytes, receivedBytes);
    }

    /**
     * this test fails most of the time
     */
    @Test
    void shouldUpload1000Files() {
        // given
        MultiValueMap<String, HttpEntity<?>> body = buildResources(1000);

        // when
        List<String> receivedBytes = sendPostRequest(body);

        // then
        assertEquals(sentBytes, receivedBytes);
    }

    private List<String> sendPostRequest(MultiValueMap<String, HttpEntity<?>> body) {
        return WebClient.builder().build().post()
                .uri("http://localhost:8080/files")
                .contentType(MediaType.MULTIPART_FORM_DATA)
                .body(BodyInserters.fromMultipartData(body))
                .retrieve()
                .bodyToMono(new ParameterizedTypeReference<List<String>>() {
                })
                .block();
    }

    private MultiValueMap<String, HttpEntity<?>> buildResources(int numberOfResources) {
        MultipartBodyBuilder builder = new MultipartBodyBuilder();
        for (int i = 0; i < numberOfResources; i++) {
            builder.part("item-" + i, buildResource(i));
        }
        return builder.build();
    }

    private ByteArrayResource buildResource(int index) {
        byte[] bytes = randomBytes();
        sentBytes.add(Base64.getEncoder().encodeToString(bytes)); // keeps track of what has been sent
        return new ByteArrayResource(bytes) {
            @Override
            public String getFilename() {
                return "filename-" + index;
            }
        };
    }

    private byte[] randomBytes() {
        byte[] bytes = new byte[ThreadLocalRandom.current().nextInt(16, 32)];
        ThreadLocalRandom.current().nextBytes(bytes);
        return bytes;
    }
}

As you can see, the only difference between these two tests is the number of files uploaded. The first test always works, and the second one doesn't work most of the time. When I was analyzing why these bytes do not match, I noticed that sometimes a few extra bytes appear at the end (something like padding).

Interestingly, when I use WebTestClient, the same problem does not occur, as shown in this test.

[rstoyanchev]

Please, edit the description to provide the full details. A link to StackOverflow is fine in addition, e.g. if there is a relevant discussion, but not as a replacement of a full description on an issue report.

[karolkrasnowski]

@rstoyanchev I updated the description with more details. I hope it looks a little better now.

[rstoyanchev]

Yes it does, thanks.

I enabled the wiretap setting of the Reactor Netty server to log request details, and also modified the test to send the Base64 encoded strings, so I could compare what was sent, to what was received on the server side, to what is observed in the FilesController after parsing. I was able to confirm the server received the same input, and that the difference shows up in FilesController, which points to an issue in the MultipartParser. Essentially on some file parts I see an additional \r\n appended, which shouldn't be there.

So I can see there is an issue, but the root cause. @poutsma, will need some help on this.

[rstoyanchev]

Here is example log output on a file part where this occurs:

2022-01-24 21:15:22.334 TRACE 221951 --- [or-http-epoll-2] o.s.h.codec.multipart.MultipartParser    : Emitting headers: [Content-Type:"text/plain;charset=ISO-8859-1", Content-Disposition:"form-data; name="item-685"; filename="filename-685"", Content-Length:"36"]
2022-01-24 21:15:22.334 TRACE 221951 --- [or-http-epoll-2] o.s.h.codec.multipart.MultipartParser    : Changed state: HEADERS -> BODY
2022-01-24 21:15:22.334 TRACE 221951 --- [or-http-epoll-2] o.s.h.codec.multipart.MultipartParser    : Emitting body: PooledSlicedByteBuf(ridx: 0, widx: 36, cap: 36/36, unwrapped: PooledUnsafeDirectByteBuf(ridx: 65530, widx: 65536, cap: 65536))
2022-01-24 21:15:22.334 TRACE 221951 --- [or-http-epoll-2] o.s.h.codec.multipart.MultipartParser    : Emitting body: PooledSlicedByteBuf(ridx: 0, widx: 2, cap: 2/2, unwrapped: PooledUnsafeDirectByteBuf(ridx: 65536, widx: 65536, cap: 65536))
2022-01-24 21:15:22.334 TRACE 221951 --- [or-http-epoll-2] o.s.h.codec.multipart.MultipartParser    : Boundary found @38 in PooledSlicedByteBuf(ridx: 0, widx: 41, cap: 41/41, unwrapped: PooledUnsafeDirectByteBuf(ridx: 43, widx: 65536, cap: 65536))

Note how the Content-Length in the part headers is 36, followed by 2 body buffers emitted. One of 36 bytes, which is the correct length of the sent item. The second of 2 bytes (content is \r\n) shouldn't be there.

[poutsma]

I will pick this one up.

[poutsma]

The problem seems to be in MultipartParser.BodyState. In a small minority of cases, the multipart boundary can spread across three buffers, whereas our current design only allows for two. As a result, the first buffer of the three is sent prematurely, even though it contains the first bytes of the boundary.

All of this occurs in BodyState::onNext, where the prevLen variable is negative when this bug occurs, see https://github.com/poutsma/spring-framework/blob/0416168d0edfaa8f321e318bd4129df7b425ee79/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java#L513,

[poutsma]

@karolkrasnowski Thanks for providing the test case, it made fixing this bug a lot easier!

[karolkrasnowski]

@poutsma no problem. Thanks for taking care of this issue!