changes f rom review

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2023-02-07 15:47:17 +11:00
parent c7e7fd837c
commit d005e975b0
12 changed files with 58 additions and 85 deletions

View File

@ -23,7 +23,6 @@ import java.util.List;
import java.util.concurrent.CompletableFuture;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.io.Retainable;
import org.eclipse.jetty.util.thread.AutoLock;
/**
@ -328,7 +327,7 @@ public class MultiPartByteRanges extends CompletableFuture<MultiPartByteRanges.P
partChunks.clear();
}
partsToFail.forEach(p -> p.fail(cause));
partChunksToFail.forEach(Retainable::release);
partChunksToFail.forEach(Content.Chunk::release);
}
}
}

View File

@ -308,7 +308,7 @@ public class MultiPartFormData extends CompletableFuture<MultiPartFormData.Parts
* <p>An ordered list of {@link MultiPart.Part}s that can
* be accessed by index or by name, or iterated over.</p>
*/
public class Parts implements Iterable<MultiPart.Part>
public class Parts implements Iterable<MultiPart.Part>, Closeable
{
private final List<MultiPart.Part> parts;
@ -378,6 +378,7 @@ public class MultiPartFormData extends CompletableFuture<MultiPartFormData.Parts
return parts.iterator();
}
@Override
public void close()
{
for (MultiPart.Part p : parts)
@ -447,19 +448,28 @@ public class MultiPartFormData extends CompletableFuture<MultiPartFormData.Parts
memoryFileSize += buffer.remaining();
if (memoryFileSize > maxMemoryFileSize)
{
// Must save to disk.
if (ensureFileChannel())
try
{
// Write existing memory chunks.
for (Content.Chunk c : partChunks)
// Must save to disk.
if (ensureFileChannel())
{
if (!write(c.getByteBuffer()))
return;
// Write existing memory chunks.
for (Content.Chunk c : partChunks)
{
write(c.getByteBuffer());
}
}
write(buffer);
if (chunk.isLast())
close();
}
write(buffer);
if (chunk.isLast())
close();
catch (Throwable x)
{
onFailure(x);
}
partChunks.forEach(Content.Chunk::release);
chunk.release();
return;
}
}
@ -469,24 +479,15 @@ public class MultiPartFormData extends CompletableFuture<MultiPartFormData.Parts
partChunks.add(chunk);
}
private boolean write(ByteBuffer buffer)
private void write(ByteBuffer buffer) throws Exception
{
try
int remaining = buffer.remaining();
while (remaining > 0)
{
int remaining = buffer.remaining();
while (remaining > 0)
{
int written = fileChannel.write(buffer);
if (written == 0)
throw new NonWritableChannelException();
remaining -= written;
}
return true;
}
catch (Throwable x)
{
onFailure(x);
return false;
int written = fileChannel.write(buffer);
if (written == 0)
throw new NonWritableChannelException();
remaining -= written;
}
}

View File

@ -132,7 +132,6 @@ public class ChunksContentSource implements Content.Source
chunksToRelease = List.copyOf(chunks);
}
}
chunksToRelease.forEach(Content.Chunk::release);
}
}

View File

@ -13,7 +13,6 @@
package org.eclipse.jetty.server;
import java.io.IOException;
import java.nio.ByteBuffer;
import org.eclipse.jetty.http.HttpFields;
@ -22,6 +21,7 @@ import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.io.Content.Chunk;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.StaticException;
/**
* A HttpStream is an abstraction that together with {@link MetaData.Request}, represents the
@ -31,6 +31,8 @@ import org.eclipse.jetty.util.Callback;
*/
public interface HttpStream extends Callback
{
Exception CONTENT_NOT_CONSUMED = new StaticException("Content Not Consumed");
/**
* <p>Attribute name to be used as a {@link Request} attribute to store/retrieve
* the {@link Connection} created during the HTTP/1.1 upgrade mechanism or the
@ -118,7 +120,7 @@ public interface HttpStream extends Callback
// if we cannot read to EOF then fail the stream rather than wait for unconsumed content
if (content == null)
return new IOException("Content not consumed");
return CONTENT_NOT_CONSUMED;
// Always release any returned content. This is a noop for EOF and Error content.
content.release();
@ -131,7 +133,7 @@ public interface HttpStream extends Callback
return null;
}
return new IOException("Content not consumed");
return CONTENT_NOT_CONSUMED;
}
class Wrapper implements HttpStream

View File

@ -228,6 +228,14 @@ public interface Request extends Attributes, Content.Source
@Override
Content.Chunk read();
/**
* Consume any available content. This bypasses any request wrappers to process the content in
* {@link Request#read()} and reads directly from the {@link HttpStream}. This reads until
* there is no content currently available or it reaches EOF.
* The {@link HttpConfiguration#setMaxUnconsumedRequestContentReads(int)} configuration can be used
* to configure how many reads will be attempted by this method.
* @return true if the content was fully consumed.
*/
boolean consumeAvailable();
/**

View File

@ -1,32 +0,0 @@
//
// ========================================================================
// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//
package org.eclipse.jetty.ee10.servlet;
import java.io.InputStream;
import java.util.List;
import jakarta.servlet.http.Part;
public class MultiPartFormInputStream
{
public MultiPartFormInputStream(InputStream inputStream, String contentType, Object o, Object o1)
{
}
public List<Part> getParts()
{
return null;
}
}

View File

@ -83,10 +83,6 @@ import org.eclipse.jetty.util.URIUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static org.eclipse.jetty.ee10.servlet.ServletContextRequest.INPUT_NONE;
import static org.eclipse.jetty.ee10.servlet.ServletContextRequest.NO_PARAMS;
import static org.eclipse.jetty.ee10.servlet.ServletContextRequest.__MULTIPART_CONFIG_ELEMENT;
/**
* The Jetty low level implementation of the ee10 {@link HttpServletRequest} object.
*
@ -101,7 +97,7 @@ public class ServletApiRequest implements HttpServletRequest
//TODO review which fields should be in ServletContextRequest
private AsyncContextState _async;
private String _characterEncoding;
private int _inputState = INPUT_NONE;
private int _inputState = ServletContextRequest.INPUT_NONE;
private BufferedReader _reader;
private String _readerEncoding;
private String _contentType;
@ -487,7 +483,7 @@ public class ServletApiRequest implements HttpServletRequest
if (contentType == null || !MimeTypes.Type.MULTIPART_FORM_DATA.is(HttpField.valueParameters(contentType, null)))
throw new ServletException("Unsupported Content-Type [%s], expected [%s]".formatted(contentType, MimeTypes.Type.MULTIPART_FORM_DATA.asString()));
MultipartConfigElement config = (MultipartConfigElement)getAttribute(__MULTIPART_CONFIG_ELEMENT);
MultipartConfigElement config = (MultipartConfigElement)getAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT);
if (config == null)
throw new IllegalStateException("No multipart config for servlet");
@ -532,7 +528,7 @@ public class ServletApiRequest implements HttpServletRequest
if (p.getSubmittedFileName() == null)
{
formContentSize = Math.addExact(formContentSize, p.getSize());
if (formContentSize > maxFormContentSize)
if (maxFormContentSize >= 0 && formContentSize > maxFormContentSize)
throw new IllegalStateException("Form is larger than max length " + maxFormContentSize);
// Servlet Spec 3.0 pg 23, parts without filename must be put into params.
@ -714,7 +710,7 @@ public class ServletApiRequest implements HttpServletRequest
@Override
public void setCharacterEncoding(String encoding) throws UnsupportedEncodingException
{
if (_inputState != INPUT_NONE)
if (_inputState != ServletContextRequest.INPUT_NONE)
return;
_characterEncoding = encoding;
@ -765,7 +761,7 @@ public class ServletApiRequest implements HttpServletRequest
@Override
public ServletInputStream getInputStream() throws IOException
{
if (_inputState != INPUT_NONE && _inputState != ServletContextRequest.INPUT_STREAM)
if (_inputState != ServletContextRequest.INPUT_NONE && _inputState != ServletContextRequest.INPUT_STREAM)
throw new IllegalStateException("READER");
_inputState = ServletContextRequest.INPUT_STREAM;
@ -811,7 +807,7 @@ public class ServletApiRequest implements HttpServletRequest
public void setContentParameters(Fields params)
{
if (params == null || params.getSize() == 0)
_contentParameters = NO_PARAMS;
_contentParameters = ServletContextRequest.NO_PARAMS;
else
_contentParameters = params;
}
@ -835,7 +831,7 @@ public class ServletApiRequest implements HttpServletRequest
// protect against calls to recycled requests (which is illegal, but
// this gives better failures
Fields parameters = _parameters;
return parameters == null ? NO_PARAMS : parameters;
return parameters == null ? ServletContextRequest.NO_PARAMS : parameters;
}
private void extractContentParameters() throws BadMessageException
@ -854,7 +850,7 @@ public class ServletApiRequest implements HttpServletRequest
try
{
int contentLength = getContentLength();
if (contentLength != 0 && _inputState == INPUT_NONE)
if (contentLength != 0 && _inputState == ServletContextRequest.INPUT_NONE)
{
String baseType = HttpField.valueParameters(getContentType(), null);
if (MimeTypes.Type.FORM_ENCODED.is(baseType) &&
@ -874,7 +870,7 @@ public class ServletApiRequest implements HttpServletRequest
}
}
else if (MimeTypes.Type.MULTIPART_FORM_DATA.is(baseType) &&
getAttribute(__MULTIPART_CONFIG_ELEMENT) != null)
getAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT) != null)
{
try
{
@ -891,7 +887,7 @@ public class ServletApiRequest implements HttpServletRequest
}
if (_contentParameters == null || _contentParameters.isEmpty())
_contentParameters = NO_PARAMS;
_contentParameters = ServletContextRequest.NO_PARAMS;
}
catch (IllegalStateException | IllegalArgumentException e)
{
@ -910,7 +906,7 @@ public class ServletApiRequest implements HttpServletRequest
{
HttpURI httpURI = _request.getHttpURI();
if (httpURI == null || StringUtil.isEmpty(httpURI.getQuery()))
_queryParameters = NO_PARAMS;
_queryParameters = ServletContextRequest.NO_PARAMS;
else
{
try
@ -1008,7 +1004,7 @@ public class ServletApiRequest implements HttpServletRequest
@Override
public BufferedReader getReader() throws IOException
{
if (_inputState != INPUT_NONE && _inputState != ServletContextRequest.INPUT_READER)
if (_inputState != ServletContextRequest.INPUT_NONE && _inputState != ServletContextRequest.INPUT_READER)
throw new IllegalStateException("STREAMED");
if (_inputState == ServletContextRequest.INPUT_READER)

View File

@ -44,7 +44,7 @@ import org.slf4j.LoggerFactory;
public class ServletContextRequest extends ContextRequest
{
public static final String __MULTIPART_CONFIG_ELEMENT = "org.eclipse.jetty.multipartConfig";
public static final String MULTIPART_CONFIG_ELEMENT = "org.eclipse.jetty.multipartConfig";
private static final Logger LOG = LoggerFactory.getLogger(ServletContextRequest.class);
static final int INPUT_NONE = 0;
static final int INPUT_STREAM = 1;

View File

@ -715,7 +715,7 @@ public class ServletHolder extends Holder<Servlet> implements Comparable<Servlet
{
MultipartConfigElement mpce = ((Registration)_registration).getMultipartConfig();
if (mpce != null)
request.setAttribute(ServletContextRequest.__MULTIPART_CONFIG_ELEMENT, mpce);
request.setAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT, mpce);
}
}

View File

@ -93,7 +93,7 @@ public class ServletMultiPartFormData
private Parts parse(ServletApiRequest request, int maxParts) throws IOException
{
MultipartConfigElement config = (MultipartConfigElement)request.getAttribute(ServletContextRequest.__MULTIPART_CONFIG_ELEMENT);
MultipartConfigElement config = (MultipartConfigElement)request.getAttribute(ServletContextRequest.MULTIPART_CONFIG_ELEMENT);
if (config == null)
throw new IllegalStateException("No multipart configuration element");

View File

@ -826,7 +826,7 @@ public class MultiPartFormInputStream
{
reset();
_numParts++;
if (_numParts >= _maxParts)
if (_maxParts >= 0 && _numParts > _maxParts)
throw new IllegalStateException(String.format("Form with too many keys [%d > %d]", _numParts, _maxParts));
}

View File

@ -2008,7 +2008,7 @@ public class Request implements HttpServletRequest
if (p.getSubmittedFileName() == null)
{
formContentSize = Math.addExact(formContentSize, p.getSize());
if (formContentSize > maxFormContentSize)
if (maxFormContentSize >= 0 && formContentSize > maxFormContentSize)
throw new IllegalStateException("Form is larger than max length " + maxFormContentSize);
// Servlet Spec 3.0 pg 23, parts without filename must be put into params.