improvements to multipart

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2023-01-16 19:07:51 +11:00
parent 3816e4b98a
commit 1e20abc902
13 changed files with 109 additions and 91 deletions

View File

@ -70,7 +70,7 @@ public class MultiPartRequestContent extends MultiPartFormData.ContentSource imp
if (headers.contains(HttpHeader.CONTENT_TYPE))
return headers;
Content.Source partContent = part.getContent();
Content.Source partContent = part.getNewContent();
if (partContent instanceof Request.Content requestContent)
{
String contentType = requestContent.getContentType();

View File

@ -129,7 +129,7 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest
int equal = contentType.lastIndexOf('=');
Charset charset = Charset.forName(contentType.substring(equal + 1));
assertEquals(encoding, charset);
assertEquals(value, Content.Source.asString(part.getContent(), charset));
assertEquals(value, Content.Source.asString(part.getNewContent(), charset));
}
});
@ -163,7 +163,7 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest
MultiPart.Part part = parts.iterator().next();
assertEquals(name, part.getName());
assertEquals("text/plain", part.getHeaders().get(HttpHeader.CONTENT_TYPE));
assertArrayEquals(data, Content.Source.asByteBuffer(part.getContent()).array());
assertArrayEquals(data, Content.Source.asByteBuffer(part.getNewContent()).array());
}
});
@ -215,8 +215,8 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest
assertEquals(name, part.getName());
assertEquals(contentType, part.getHeaders().get(HttpHeader.CONTENT_TYPE));
assertEquals(fileName, part.getFileName());
assertEquals(data.length, part.getContent().getLength());
assertArrayEquals(data, Content.Source.asByteBuffer(part.getContent()).array());
assertEquals(data.length, part.getNewContent().getLength());
assertArrayEquals(data, Content.Source.asByteBuffer(part.getNewContent()).array());
}
});
@ -271,8 +271,8 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest
assertEquals(name, part.getName());
assertEquals(contentType, part.getHeaders().get(HttpHeader.CONTENT_TYPE));
assertEquals(tmpPath.getFileName().toString(), part.getFileName());
assertEquals(Files.size(tmpPath), part.getContent().getLength());
assertEquals(data, Content.Source.asString(part.getContent(), encoding));
assertEquals(Files.size(tmpPath), part.getNewContent().getLength());
assertEquals(data, Content.Source.asString(part.getNewContent(), encoding));
}
});
@ -323,14 +323,14 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest
assertEquals(field, fieldPart.getName());
assertEquals(contentType, fieldPart.getHeaders().get(HttpHeader.CONTENT_TYPE));
assertEquals(value, Content.Source.asString(fieldPart.getContent(), encoding));
assertEquals(value, Content.Source.asString(fieldPart.getNewContent(), encoding));
assertEquals(headerValue, fieldPart.getHeaders().get(headerName));
assertEquals(fileField, filePart.getName());
assertEquals("application/octet-stream", filePart.getHeaders().get(HttpHeader.CONTENT_TYPE));
assertEquals(tmpPath.getFileName().toString(), filePart.getFileName());
assertEquals(Files.size(tmpPath), filePart.getContent().getLength());
assertArrayEquals(data, Content.Source.asByteBuffer(filePart.getContent()).array());
assertEquals(Files.size(tmpPath), filePart.getNewContent().getLength());
assertArrayEquals(data, Content.Source.asByteBuffer(filePart.getNewContent()).array());
}
});
@ -367,11 +367,11 @@ public class MultiPartRequestContentTest extends AbstractHttpClientServerTest
MultiPart.Part fieldPart = parts.get(0);
MultiPart.Part filePart = parts.get(1);
assertEquals(value, Content.Source.asString(fieldPart.getContent(), encoding));
assertEquals(value, Content.Source.asString(fieldPart.getNewContent(), encoding));
assertEquals("file", filePart.getName());
assertEquals("application/octet-stream", filePart.getHeaders().get(HttpHeader.CONTENT_TYPE));
assertEquals("fileName", filePart.getFileName());
assertArrayEquals(fileData, Content.Source.asByteBuffer(filePart.getContent()).array());
assertArrayEquals(fileData, Content.Source.asByteBuffer(filePart.getNewContent()).array());
}
});

View File

@ -126,6 +126,7 @@ public class MultiPart
private final HttpFields fields;
protected Path path;
private boolean temporary = true;
private Content.Source content;
public Part(String name, String fileName, HttpFields fields)
{
@ -162,7 +163,8 @@ public class MultiPart
}
/**
* <p>Returns the content of this part.</p>
* <p>Returns the content of this part as a {@link Content.Source}.</p>
* <p>Calling this method multiple times will return the same instance.</p>
* <p>The content type and content encoding are specified in this part's
* {@link #getHeaders() headers}.</p>
* <p>The content encoding may be specified by the part named {@code _charset_},
@ -171,11 +173,30 @@ public class MultiPart
*
* @return the content of this part
*/
public abstract Content.Source getContent();
public Content.Source getContentSource()
{
if (content == null)
content = getNewContent();
return content;
}
/**
* <p>Returns the content of this part as a new {@link Content.Source}</p>
* <p>If the content is reproducible, invoking this method multiple times will return
* a different independent instance for every invocation.</p>
* <p>If the content is not reproducible, subsequent calls to this method will return null.</p>
* <p>The content type and content encoding are specified in this part's {@link #getHeaders() headers}.</p>
* <p>The content encoding may be specified by the part named {@code _charset_},
* as specified in
* <a href="https://datatracker.ietf.org/doc/html/rfc7578#section-4.6">RFC 7578, section 4.6</a>.</p>
*
* @return the content of this part as a new {@link Content.Source}
*/
public abstract Content.Source getNewContent();
public long getLength()
{
return getContent().getLength();
return getNewContent().getLength();
}
/**
@ -200,7 +221,7 @@ public class MultiPart
Charset charset = defaultCharset != null ? defaultCharset : UTF_8;
if (charsetName != null)
charset = Charset.forName(charsetName);
return Content.Source.asString(getContent(), charset);
return Content.Source.asString(getNewContent(), charset);
}
catch (IOException x)
{
@ -229,12 +250,11 @@ public class MultiPart
*/
public void writeTo(Path path) throws IOException
{
if (this.path == null)
{
try (OutputStream out = Files.newOutputStream(path))
{
IO.copy(Content.Source.asInputStream(getContent()), out);
IO.copy(Content.Source.asInputStream(getNewContent()), out);
}
this.path = path;
}
@ -251,17 +271,11 @@ public class MultiPart
Files.delete(this.path);
}
public void close(Throwable failure) throws IOException
public void close() throws IOException
{
// TODO: why do we need this, no one ever uses the failure??
if (temporary)
delete();
}
public final void close() throws IOException
{
close(null);
}
}
/**
@ -292,7 +306,7 @@ public class MultiPart
}
@Override
public Content.Source getContent()
public Content.Source getNewContent()
{
return new ByteBufferContentSource(content);
}
@ -332,15 +346,15 @@ public class MultiPart
}
@Override
public Content.Source getContent()
public Content.Source getNewContent()
{
return new ChunksContentSource(content.stream().map(Content.Chunk::slice).toList());
}
@Override
public void close(Throwable failure) throws IOException
public void close() throws IOException
{
super.close(failure);
super.close();
content.forEach(Retainable::release);
}
@ -382,7 +396,7 @@ public class MultiPart
}
@Override
public Content.Source getContent()
public Content.Source getNewContent()
{
return new PathContentSource(path);
}
@ -414,7 +428,7 @@ public class MultiPart
}
@Override
public Content.Source getContent()
public Content.Source getNewContent()
{
Content.Source c = content;
content = null;
@ -616,7 +630,7 @@ public class MultiPart
else
{
part = parts.poll();
partContent = part.getContent();
partContent = part.getNewContent();
state = State.HEADERS;
yield Content.Chunk.from(firstBoundary.slice(), false);
}
@ -643,7 +657,7 @@ public class MultiPart
else
{
part = parts.poll();
partContent = part.getContent();
partContent = part.getNewContent();
state = State.HEADERS;
yield Content.Chunk.from(middleBoundary.slice(), false);
}
@ -765,7 +779,7 @@ public class MultiPart
{
try
{
part.close(failure);
part.close();
}
catch (IOException e)
{

View File

@ -265,7 +265,7 @@ public class MultiPartByteRanges extends CompletableFuture<MultiPartByteRanges.P
}
@Override
public Content.Source getContent()
public Content.Source getNewContent()
{
return new PathContentSource(path, byteRange);
}
@ -328,7 +328,7 @@ public class MultiPartByteRanges extends CompletableFuture<MultiPartByteRanges.P
{
try
{
part.close(cause);
part.close();
}
catch (IOException e)
{

View File

@ -68,7 +68,7 @@ import static java.nio.charset.StandardCharsets.US_ASCII;
*
* @see Parts
*/
public class MultiPartFormData extends CompletableFuture<MultiPartFormData.Parts>
public class MultiPartFormData extends CompletableFuture<MultiPartFormData.Parts> implements Closeable
{
private static final Logger LOG = LoggerFactory.getLogger(MultiPartFormData.class);
@ -569,7 +569,7 @@ public class MultiPartFormData extends CompletableFuture<MultiPartFormData.Parts
{
try
{
part.close(cause);
part.close();
}
catch (IOException e)
{
@ -629,4 +629,25 @@ public class MultiPartFormData extends CompletableFuture<MultiPartFormData.Parts
}
}
}
@Override
public void close()
{
// TODO: Can we do this async?
MultiPartFormData.Parts parts = getNow(null);
if (parts != null)
{
for (MultiPart.Part p : parts)
{
try
{
p.close();
}
catch (Throwable e)
{
LOG.warn("Errors deleting multipart tmp files", e);
}
}
}
}
}

View File

@ -245,7 +245,7 @@ public class MultiPartCaptureTest
List<MultiPart.Part> charSetParts = allParts.get("_charset_");
if (charSetParts != null)
{
defaultCharset = Promise.Completable.<String>with(p -> Content.Source.asString(charSetParts.get(0).getContent(), StandardCharsets.US_ASCII, p))
defaultCharset = Promise.Completable.<String>with(p -> Content.Source.asString(charSetParts.get(0).getNewContent(), StandardCharsets.US_ASCII, p))
.get();
}
@ -255,8 +255,8 @@ public class MultiPartCaptureTest
assertThat("Part[" + expected.name + "]", parts, is(notNullValue()));
MultiPart.Part part = parts.get(0);
String charset = getCharsetFromContentType(part.getHeaders().get(HttpHeader.CONTENT_TYPE), defaultCharset);
assertTrue(part.getContent().rewind());
String partContent = Content.Source.asString(part.getContent(), Charset.forName(charset));
assertTrue(part.getNewContent().rewind());
String partContent = Content.Source.asString(part.getNewContent(), Charset.forName(charset));
assertThat("Part[" + expected.name + "].contents", partContent, containsString(expected.value));
}
@ -276,8 +276,8 @@ public class MultiPartCaptureTest
assertThat("Part[" + expected.name + "]", parts, is(notNullValue()));
MultiPart.Part part = parts.get(0);
MessageDigest digest = MessageDigest.getInstance("SHA1");
assertTrue(part.getContent().rewind());
try (InputStream partInputStream = Content.Source.asInputStream(part.getContent());
assertTrue(part.getNewContent().rewind());
try (InputStream partInputStream = Content.Source.asInputStream(part.getNewContent());
DigestOutputStream digester = new DigestOutputStream(OutputStream.nullOutputStream(), digest))
{
IO.copy(partInputStream, digester);

View File

@ -189,25 +189,25 @@ public class MultiPartFormDataTest
MultiPart.Part fileName = parts.getFirst("fileName");
assertThat(fileName, notNullValue());
Content.Source partContent = fileName.getContent();
Content.Source partContent = fileName.getNewContent();
assertThat(partContent.getLength(), is(3L));
assertThat(Content.Source.asString(partContent), is("abc"));
MultiPart.Part desc = parts.getFirst("desc");
assertThat(desc, notNullValue());
partContent = desc.getContent();
partContent = desc.getNewContent();
assertThat(partContent.getLength(), is(3L));
assertThat(Content.Source.asString(partContent), is("123"));
MultiPart.Part title = parts.getFirst("title");
assertThat(title, notNullValue());
partContent = title.getContent();
partContent = title.getNewContent();
assertThat(partContent.getLength(), is(3L));
assertThat(Content.Source.asString(partContent), is("ttt"));
MultiPart.Part datafile = parts.getFirst("datafile5239138112980980385.txt");
assertThat(datafile, notNullValue());
partContent = datafile.getContent();
partContent = datafile.getNewContent();
assertThat(partContent.getLength(), is(3L));
assertThat(Content.Source.asString(partContent), is("000"));
}
@ -275,11 +275,11 @@ public class MultiPartFormDataTest
assertThat(parts.size(), is(2));
MultiPart.Part part1 = parts.getFirst("field1");
assertThat(part1, notNullValue());
Content.Source partContent = part1.getContent();
Content.Source partContent = part1.getNewContent();
assertThat(Content.Source.asString(partContent), is("Joe Blow"));
MultiPart.Part part2 = parts.getFirst("stuff");
assertThat(part2, notNullValue());
partContent = part2.getContent();
partContent = part2.getNewContent();
assertThat(Content.Source.asString(partContent), is("aaaabbbbb"));
}
@ -312,7 +312,7 @@ public class MultiPartFormDataTest
assertThat(parts.size(), is(1));
MultiPart.Part part2 = parts.getFirst("stuff");
assertThat(part2, notNullValue());
Content.Source partContent = part2.getContent();
Content.Source partContent = part2.getNewContent();
assertThat(Content.Source.asString(partContent), is("aaaabbbbb"));
}
@ -340,7 +340,7 @@ public class MultiPartFormDataTest
assertThat(part, instanceOf(MultiPart.PathPart.class));
MultiPart.PathPart pathPart = (MultiPart.PathPart)part;
assertTrue(Files.exists(pathPart.getPath()));
assertEquals("ABCDEFGHIJKLMNOPQRSTUVWXYZ", Content.Source.asString(part.getContent()));
assertEquals("ABCDEFGHIJKLMNOPQRSTUVWXYZ", Content.Source.asString(part.getNewContent()));
}
@Test
@ -422,13 +422,13 @@ public class MultiPartFormDataTest
MultiPart.Part part1 = parts.get(0);
assertThat(part1, instanceOf(MultiPart.ChunksPart.class));
assertEquals(chunk, Content.Source.asString(part1.getContent()));
assertEquals(chunk, Content.Source.asString(part1.getNewContent()));
MultiPart.Part part2 = parts.get(1);
assertThat(part2, instanceOf(MultiPart.PathPart.class));
MultiPart.PathPart pathPart2 = (MultiPart.PathPart)part2;
assertTrue(Files.exists(pathPart2.getPath()));
assertEquals(chunk.repeat(4), Content.Source.asString(part2.getContent()));
assertEquals(chunk.repeat(4), Content.Source.asString(part2.getNewContent()));
}
@Test

View File

@ -361,11 +361,11 @@ public class MultiPartTest
MultiPart.Part part1 = listener.parts.get(0);
assertEquals("value", part1.getHeaders().get("name"));
assertEquals("Hello", Content.Source.asString(part1.getContent()));
assertEquals("Hello", Content.Source.asString(part1.getNewContent()));
MultiPart.Part part2 = listener.parts.get(1);
assertEquals("9001", part2.getHeaders().get("powerLevel"));
assertEquals("secondary\r\ncontent", Content.Source.asString(part2.getContent()));
assertEquals("secondary\r\ncontent", Content.Source.asString(part2.getNewContent()));
assertEquals(0, data.remaining());
}
@ -397,11 +397,11 @@ public class MultiPartTest
MultiPart.Part part1 = listener.parts.get(0);
assertEquals("value", part1.getHeaders().get("name"));
assertEquals("Hello", Content.Source.asString(part1.getContent()));
assertEquals("Hello", Content.Source.asString(part1.getNewContent()));
MultiPart.Part part2 = listener.parts.get(1);
assertEquals("9001", part2.getHeaders().get("powerLevel"));
assertEquals("secondary\ncontent", Content.Source.asString(part2.getContent()));
assertEquals("secondary\ncontent", Content.Source.asString(part2.getNewContent()));
assertEquals(0, data.remaining());
}
@ -457,7 +457,7 @@ public class MultiPartTest
assertEquals(1, listener.parts.size());
MultiPart.Part part = listener.parts.get(0);
assertEquals("value", part.getHeaders().get("name"));
assertEquals("", Content.Source.asString(part.getContent()));
assertEquals("", Content.Source.asString(part.getNewContent()));
}
@Test
@ -477,7 +477,7 @@ public class MultiPartTest
assertEquals(1, listener.parts.size());
MultiPart.Part part = listener.parts.get(0);
assertEquals("value", part.getHeaders().get("name"));
assertEquals("", Content.Source.asString(part.getContent()));
assertEquals("", Content.Source.asString(part.getNewContent()));
}
@Test
@ -508,7 +508,7 @@ public class MultiPartTest
assertEquals(1, listener.parts.size());
MultiPart.Part part = listener.parts.get(0);
assertEquals("value", part.getHeaders().get("name"));
assertThat(Content.Source.asString(part.getContent()), is("""
assertThat(Content.Source.asString(part.getNewContent()), is("""
Hello\r
this is not a --BOUNDARY\r
that's a boundary"""));
@ -532,7 +532,7 @@ public class MultiPartTest
assertThat(epilogueBuffer.remaining(), is(0));
assertEquals(1, listener.parts.size());
MultiPart.Part part = listener.parts.get(0);
assertThat(Content.Source.asByteBuffer(part.getContent()), is(ByteBuffer.wrap(random)));
assertThat(Content.Source.asByteBuffer(part.getNewContent()), is(ByteBuffer.wrap(random)));
}
@Test
@ -556,7 +556,7 @@ public class MultiPartTest
assertEquals(1, listener.parts.size());
MultiPart.Part part = listener.parts.get(0);
assertEquals("value", part.getHeaders().get("name"));
assertEquals("Hello", Content.Source.asString(part.getContent()));
assertEquals("Hello", Content.Source.asString(part.getNewContent()));
}
@Test

View File

@ -34,7 +34,6 @@ import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http.MultiPart;
import org.eclipse.jetty.http.MultiPartFormData;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.http.Trailers;
@ -652,23 +651,7 @@ public class HttpChannelState implements HttpChannel, Components
// Clean up any multipart tmp files and release any associated resources.
MultiPartFormData multiParts = (MultiPartFormData)_request.getAttribute(MultiPartFormData.class.getName());
if (multiParts != null)
{
MultiPartFormData.Parts parts = multiParts.getNow(null);
if (parts != null)
{
for (MultiPart.Part p : parts)
{
try
{
p.close();
}
catch (Throwable e)
{
LOG.warn("Errors deleting multipart tmp files", e);
}
}
}
}
multiParts.close();
}
finally
{

View File

@ -119,11 +119,11 @@ public class MultiPartByteRangesTest
assertEquals(3, parts.size());
MultiPart.Part part1 = parts.get(0);
assertEquals("12", Content.Source.asString(part1.getContent()));
assertEquals("12", Content.Source.asString(part1.getNewContent()));
MultiPart.Part part2 = parts.get(1);
assertEquals("456", Content.Source.asString(part2.getContent()));
assertEquals("456", Content.Source.asString(part2.getNewContent()));
MultiPart.Part part3 = parts.get(2);
assertEquals("CDEF", Content.Source.asString(part3.getContent()));
assertEquals("CDEF", Content.Source.asString(part3.getNewContent()));
}
}
}

View File

@ -80,7 +80,7 @@ public class MultiPartFormDataHandlerTest
.whenComplete((parts, failure) ->
{
if (parts != null)
Content.copy(parts.get(0).getContent(), response, callback);
Content.copy(parts.get(0).getNewContent(), response, callback);
else
Response.writeError(request, response, callback, failure);
});
@ -129,7 +129,7 @@ public class MultiPartFormDataHandlerTest
MultiPartFormData formData = (MultiPartFormData)request.getAttribute(MultiPartFormData.class.getName());
assertNotNull(formData);
MultiPart.Part part = formData.get().get(0);
Content.copy(part.getContent(), response, callback);
Content.copy(part.getNewContent(), response, callback);
return true;
}
});
@ -321,7 +321,7 @@ public class MultiPartFormDataHandlerTest
HttpFields headers2 = part2.getHeaders();
assertEquals(2, headers2.size());
assertEquals("application/octet-stream", headers2.get(HttpHeader.CONTENT_TYPE));
assertEquals(32, part2.getContent().getLength());
assertEquals(32, part2.getNewContent().getLength());
}
}
}

View File

@ -181,10 +181,10 @@ public class ResourceHandlerByteRangesTest
assertEquals(2, parts.size());
MultiPart.Part part1 = parts.get(0);
assertEquals("text/plain", part1.getHeaders().get(HttpHeader.CONTENT_TYPE));
assertEquals("234", Content.Source.asString(part1.getContent()));
assertEquals("234", Content.Source.asString(part1.getNewContent()));
MultiPart.Part part2 = parts.get(1);
assertEquals("text/plain", part2.getHeaders().get(HttpHeader.CONTENT_TYPE));
assertEquals("xyz", Content.Source.asString(part2.getContent()));
assertEquals("xyz", Content.Source.asString(part2.getNewContent()));
}
}
}

View File

@ -76,7 +76,7 @@ public class ServletMultiPartFormData
{
try
{
// Look for a previously read and parsed MultiPartFormData from the DelayedHandler
// Look for a previously read and parsed MultiPartFormData from the DelayedHandler.
MultiPartFormData formData = (MultiPartFormData)request.getAttribute(MultiPartFormData.class.getName());
if (formData != null)
return new Parts(formData);
@ -127,7 +127,7 @@ public class ServletMultiPartFormData
{
ByteBuffer buffer = byteBufferPool.newByteBuffer(bufferSize, false);
boolean readEof = false;
while (BufferUtil.space(buffer) > 0)
while (BufferUtil.space(buffer) > bufferSize / 2)
{
int read = BufferUtil.readFrom(input, buffer);
if (read < 0)
@ -190,7 +190,7 @@ public class ServletMultiPartFormData
@Override
public InputStream getInputStream() throws IOException
{
return Content.Source.asInputStream(_part.getContent());
return Content.Source.asInputStream(_part.getNewContent());
}
@Override