Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling .hasNext() or .next() on MultiPart Iterator after the first call to .next() causes NullPointerException #8358

Closed
brain-dev-null opened this issue Feb 9, 2024 · 1 comment · Fixed by #8375
Assignees
Labels
4.x Version 4.x bug Something isn't working media-support Media type processing in SE P2 SE webserver
Milestone

Comments

@brain-dev-null
Copy link

When iterating over the parts of a multipart request using the io.helidon.http.media.multipart.MultiPart iterator, calliing .hasNext() of .next() after the first call to .next() causes a NullPointerException to be thrown.

Environment Details

  • Helidon Version: 4.0.5
  • Helidon SE
  • JDK version: 21
  • OS: Linux x86
  • Docker version (if applicable): /

Problem Description

Current behavior: Iterating over parts of a multipart requests fails when calling .hasNext() or .next() on io.helidon.http.media.multipart.MultiPart after .next() was called once.

Expected behavior: Calling .hasNext() should return true as long as unread parts remain and a subsequent call to .next() should return the next io.helidon.http.media.multipart.ReadablePart instance from the request. If no more parts remain, .hasNext() should return false.

Theis can be reproduced by running the Helidon SE MultiPart Example or the minimal example shown below.

When I did this, the following stacktrace was printed:

io.helidon.http.RequestException: Cannot invoke "io.helidon.common.buffers.BufferData.available()" because "this.nextBuffer" is null
	at io.helidon.http.RequestException$Builder.build(RequestException.java:139)
	at io.helidon.webserver.http.ErrorHandlers.unhandledError(ErrorHandlers.java:202)
	at io.helidon.webserver.http.ErrorHandlers.lambda$handleError$1(ErrorHandlers.java:182)
	at java.base/java.util.Optional.ifPresentOrElse(Optional.java:198)
	at io.helidon.webserver.http.ErrorHandlers.handleError(ErrorHandlers.java:181)
	at io.helidon.webserver.http.ErrorHandlers.runWithErrorHandling(ErrorHandlers.java:118)
	at io.helidon.webserver.http.Filters.filter(Filters.java:77)
	at io.helidon.webserver.http.HttpRouting.route(HttpRouting.java:109)
	at io.helidon.webserver.http1.Http1Connection.route(Http1Connection.java:396)
	at io.helidon.webserver.http1.Http1Connection.handle(Http1Connection.java:169)
	at io.helidon.webserver.ConnectionHandler.run(ConnectionHandler.java:155)
	at io.helidon.common.task.InterruptableTask.call(InterruptableTask.java:47)
	at io.helidon.webserver.ThreadPerTaskExecutor$ThreadBoundFuture.run(ThreadPerTaskExecutor.java:239)
	at java.base/java.lang.VirtualThread.run(VirtualThread.java:311)
Caused by: java.lang.NullPointerException: Cannot invoke "io.helidon.common.buffers.BufferData.available()" because "this.nextBuffer" is null
	at io.helidon.http.media.multipart.ReadablePartNoLength$PartInputStream.finish(ReadablePartNoLength.java:198)
	at io.helidon.http.media.multipart.ReadablePartNoLength.finish(ReadablePartNoLength.java:77)
	at io.helidon.http.media.multipart.MultiPartImpl.hasNext(MultiPartImpl.java:70)
	at io.helidon.examples.media.multipart.FileService.upload(FileService.java:136)
	at io.helidon.webserver.http.HttpRouting$RoutingExecutor.doRoute(HttpRouting.java:668)
	at io.helidon.webserver.http.HttpRouting$RoutingExecutor.call(HttpRouting.java:627)
	at io.helidon.webserver.http.HttpRouting$RoutingExecutor.call(HttpRouting.java:605)
	at io.helidon.webserver.http.ErrorHandlers.runWithErrorHandling(ErrorHandlers.java:75)
	... 8 more

Steps to reproduce

  1. Create a new Project using this code snippet and run it
    import io.helidon.http.media.multipart.MultiPart;
    import io.helidon.webserver.WebServer;
    
    public class Main {
        public static void main(String[] args) {
            WebServer.builder()
                    .port(8080)
                    .routing(routing -> routing.post("/", (serverRequest, serverResponse) -> {
                        var parts = serverRequest.content().as(MultiPart.class);
                        while (parts.hasNext()) {
                            var part = parts.next();
                            System.out.println(part.name());
                        }
                        serverResponse.status(200).send();
                    })).build().start();
            while (true) {}
        }
    }
  2. Send a POST multipart request with more than 2 parts, e.g. using:
    curl --request POST 'http://localhost:8080' --form 'field_1=a' --form 'field_2=b'

The expected behavior of this procedure would be that the following output is printed:

field_1
field_2

The actual behavior is that only the first line including field_1 is printed, then the NullPointerException is thrown.

@brain-dev-null brain-dev-null changed the title Calling .hasNext() or .next() on MultiPart Iterator after a one call to .next() causes NullPointerException Calling .hasNext() or .next() on MultiPart Iterator after the first call to .next() causes NullPointerException Feb 9, 2024
@romain-grecourt romain-grecourt added bug Something isn't working SE webserver 4.x Version 4.x media-support Media type processing in SE labels Feb 9, 2024
@m0mus m0mus added the P2 label Feb 12, 2024
@tomas-langer
Copy link
Member

Reproduced - requires streaming (no Content-Length defined) multipart parts, which we did not test.
Added test to make sure both approaches work.
Fixed an additional problem, where calling consume would fail with an exception.

@romain-grecourt romain-grecourt added this to the 4.0.6 milestone Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x bug Something isn't working media-support Media type processing in SE P2 SE webserver
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants