diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpContent.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpContent.java index 17c8d3ab1bd..ce1e0a82c25 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpContent.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpContent.java @@ -26,6 +26,7 @@ import java.util.Iterator; import org.eclipse.jetty.client.api.ContentProvider; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -218,15 +219,8 @@ public class HttpContent implements Callback, Closeable @Override public void close() { - try - { - if (iterator instanceof Closeable) - ((Closeable)iterator).close(); - } - catch (Throwable x) - { - LOG.ignore(x); - } + if (iterator instanceof Closeable) + IO.close((Closeable)iterator); } @Override diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/MultiPartContentProvider.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/MultiPartContentProvider.java index e6441371d57..30d817dfc81 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/MultiPartContentProvider.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/MultiPartContentProvider.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -345,10 +346,16 @@ public class MultiPartContentProvider extends AbstractTypedContentProvider imple if (iterator.hasNext()) return iterator.next(); ++index; - if (index == parts.size()) - state = State.LAST_BOUNDARY; - else + if (index < parts.size()) + { state = State.MIDDLE_BOUNDARY; + if (iterator instanceof Closeable) + IO.close((Closeable)iterator); + } + else + { + state = State.LAST_BOUNDARY; + } break; } case MIDDLE_BOUNDARY: @@ -380,14 +387,14 @@ public class MultiPartContentProvider extends AbstractTypedContentProvider imple @Override public void succeeded() { - if (iterator instanceof Callback) + if (state == State.CONTENT && iterator instanceof Callback) ((Callback)iterator).succeeded(); } @Override public void failed(Throwable x) { - if (iterator instanceof Callback) + if (state == State.CONTENT && iterator instanceof Callback) ((Callback)iterator).failed(x); } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java index e1d855e3e2b..ec90c842c31 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.client.util; import java.io.BufferedWriter; import java.io.ByteArrayInputStream; +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.OutputStream; @@ -31,10 +32,12 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.MultipartConfigElement; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -435,6 +438,46 @@ public class MultiPartContentProviderTest extends AbstractHttpClientServerTest assertTrue(responseLatch.await(5, TimeUnit.SECONDS)); } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testEachPartIsClosed(Scenario scenario) throws Exception + { + String name1 = "field1"; + String value1 = "value1"; + String name2 = "field2"; + String value2 = "value2"; + start(scenario, new AbstractMultiPartHandler() + { + @Override + protected void handle(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + Collection parts = request.getParts(); + assertEquals(2, parts.size()); + Iterator iterator = parts.iterator(); + Part part1 = iterator.next(); + assertEquals(name1, part1.getName()); + assertEquals(value1, IO.toString(part1.getInputStream())); + Part part2 = iterator.next(); + assertEquals(name2, part2.getName()); + assertEquals(value2, IO.toString(part2.getInputStream())); + } + }); + + AtomicInteger closeCount = new AtomicInteger(); + MultiPartContentProvider multiPart = new MultiPartContentProvider(); + multiPart.addFieldPart(name1, new CloseableStringContentProvider(value1, closeCount::incrementAndGet), null); + multiPart.addFieldPart(name2, new CloseableStringContentProvider(value2, closeCount::incrementAndGet), null); + multiPart.close(); + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .method(HttpMethod.POST) + .content(multiPart) + .send(); + + assertEquals(200, response.getStatus()); + assertEquals(2, closeCount.get()); + } + private abstract static class AbstractMultiPartHandler extends AbstractHandler { @Override @@ -448,4 +491,49 @@ public class MultiPartContentProviderTest extends AbstractHttpClientServerTest protected abstract void handle(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException; } + + private static class CloseableStringContentProvider extends StringContentProvider + { + private final Runnable closeFn; + + private CloseableStringContentProvider(String content, Runnable closeFn) + { + super(content); + this.closeFn = closeFn; + } + + @Override + public Iterator iterator() + { + return new CloseableIterator<>(super.iterator()); + } + + private class CloseableIterator implements Iterator, Closeable + { + private final Iterator iterator; + + public CloseableIterator(Iterator iterator) + { + this.iterator = iterator; + } + + @Override + public boolean hasNext() + { + return iterator.hasNext(); + } + + @Override + public T next() + { + return iterator.next(); + } + + @Override + public void close() + { + closeFn.run(); + } + } + } }