Skip to content

Commit

Permalink
Issue #12695 - don't return ByteBuffer from .toMappedBuffer if FS rep…
Browse files Browse the repository at this point in the history
…orts as UnsupportedOperationException. (#12696)

* restore FileBufferedResponseHandlerTest
* Remove custom IteratingCallback
* Use Content.Source.from(Path)
* New Interceptor based Content.Sink
  • Loading branch information
joakime authored Jan 15, 2025
1 parent 075575c commit e89d1e6
Show file tree
Hide file tree
Showing 5 changed files with 773 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -262,17 +262,25 @@ public static Response parseResponse(Input input) throws IOException
return parseResponse(input, false);
}

public static Response parseResponse(Input input, Response response) throws IOException
{
return parseResponse(input, response, false);
}

public static Response parseResponse(Input input, boolean head) throws IOException
{
Response response;
return parseResponse(input, new Response(), head);
}

public static Response parseResponse(Input input, Response response, boolean head) throws IOException
{
HttpParser parser = input.takeHttpParser();
if (parser != null)
{
response = (Response)parser.getHandler();
}
else
{
response = new Response();
parser = new HttpParser(response);
}
parser.setHeadResponse(head);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ private SingleBufferFileMappedHttpContent(HttpContent content) throws IOExceptio
if (path == null)
throw new IOException("Cannot memory map Content whose Resource is not backed by a Path: " + content.getResource());
_buffer = BufferUtil.toMappedBuffer(path);
if (_buffer == null)
throw new IOException("Cannot memory map Content (not supported by underlying FileSystem): " + content.getResource());
_contentLength = new HttpField(HttpHeader.CONTENT_LENGTH, Integer.toString(_buffer.remaining()));
_lastModified = content.getLastModified();
_lastModifiedInstant = content.getLastModifiedInstant();
Expand Down Expand Up @@ -176,6 +178,8 @@ private MultiBufferFileMappedHttpContent(HttpContent content, int maxBufferSize)
{
long len = Math.min(contentLength - currentPos, maxBufferSize);
_buffers[i] = BufferUtil.toMappedBuffer(path, currentPos, len);
if (_buffers[i] == null)
throw new IOException("Cannot memory map Content (not supported by underlying FileSystem): " + content.getResource());
currentPos += len;
total += _buffers[i].remaining();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
import java.nio.Buffer;
import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
Expand Down Expand Up @@ -1166,19 +1167,48 @@ public static ByteBuffer toDirectBuffer(String s, Charset charset)
return buf;
}

/**
* Attempt to return a ByteBuffer created via {@link FileChannel#map} call.
*
* @param path the path to map
* @return the mapped bytebuffer, or null if unable to map the {@link FileChannel}
* @throws IOException if unable to open mapped byte buffer
*/
public static ByteBuffer toMappedBuffer(Path path) throws IOException
{
return toMappedBuffer(path, 0, Files.size(path));
}

/**
* Attempt to return a ByteBuffer created via {@link FileChannel#map} call.
*
* @param filePath the path to map
* @param pos the position to start from
* @param len the length to support
* @return the mapped bytebuffer, or null if unable to map the {@link FileChannel}
* @throws IOException if unable to open mapped byte buffer
*/
public static ByteBuffer toMappedBuffer(Path filePath, long pos, long len) throws IOException
{
try (FileChannel channel = FileChannel.open(filePath, StandardOpenOption.READ))
{
return channel.map(MapMode.READ_ONLY, pos, len);
}
catch (UnsupportedEncodingException e)
{
if (LOG.isTraceEnabled())
LOG.trace("Ignored exception", e);
return null;
}
}

/**
* Attempt to return a ByteBuffer created via {@link FileChannel#map} call.
*
* @param resource the resource to map
* @return the mapped bytebuffer, or null if unable to map the {@link FileChannel} (usually due to Resource not supporting memory mapped files)
* @throws IOException if unable to open mapped byte buffer
*/
public static ByteBuffer toMappedBuffer(Resource resource) throws IOException
{
Path path = resource.getPath();
Expand All @@ -1187,6 +1217,15 @@ public static ByteBuffer toMappedBuffer(Resource resource) throws IOException
return toMappedBuffer(path);
}

/**
* Attempt to return a ByteBuffer created via {@link FileChannel#map} call.
*
* @param resource the resource to map
* @param pos the position to start from
* @param len the length to support
* @return the mapped bytebuffer, or null if unable to map the {@link FileChannel} (usually due to Resource not supporting memory mapped files)
* @throws IOException if unable to open mapped byte buffer
*/
public static ByteBuffer toMappedBuffer(Resource resource, long pos, long len) throws IOException
{
Path path = resource.getPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
import java.util.Objects;

import org.eclipse.jetty.ee9.nested.HttpOutput.Interceptor;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.IteratingCallback;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -52,6 +53,8 @@ public class FileBufferedResponseHandler extends BufferedResponseHandler
{
private static final Logger LOG = LoggerFactory.getLogger(FileBufferedResponseHandler.class);

private static final int DEFAULT_BUFFER_SIZE = 64 * 1024;
private int _bufferSize = DEFAULT_BUFFER_SIZE;
private Path _tempDir = new File(System.getProperty("java.io.tmpdir")).toPath();

public Path getTempDir()
Expand All @@ -64,6 +67,16 @@ public void setTempDir(Path tempDir)
_tempDir = Objects.requireNonNull(tempDir);
}

public int getBufferSize()
{
return _bufferSize;
}

public void setBufferSize(int bufferSize)
{
this._bufferSize = bufferSize;
}

@Override
protected BufferedInterceptor newBufferedInterceptor(HttpChannel httpChannel, Interceptor interceptor)
{
Expand All @@ -72,8 +85,6 @@ protected BufferedInterceptor newBufferedInterceptor(HttpChannel httpChannel, In

class FileBufferedInterceptor implements BufferedResponseHandler.BufferedInterceptor
{
private static final int MAX_MAPPED_BUFFER_SIZE = Integer.MAX_VALUE / 2;

private final Interceptor _next;
private final HttpChannel _channel;
private Boolean _aggregating;
Expand Down Expand Up @@ -193,41 +204,11 @@ private void commit(Callback callback)
}

// Create an iterating callback to do the writing
IteratingCallback icb = new IteratingCallback()
{
private final long fileLength = _filePath.toFile().length();
private long _pos = 0;
private boolean _last = false;

@Override
protected Action process() throws Exception
{
if (_last)
return Action.SUCCEEDED;

long len = Math.min(MAX_MAPPED_BUFFER_SIZE, fileLength - _pos);
_last = (_pos + len == fileLength);
ByteBuffer buffer = BufferUtil.toMappedBuffer(_filePath, _pos, len);
getNextInterceptor().write(buffer, _last, this);
_pos += len;
return Action.SCHEDULED;
}

@Override
protected void onCompleteSuccess()
{
dispose();
callback.succeeded();
}

@Override
protected void onFailure(Throwable cause)
{
dispose();
callback.failed(cause);
}
};
icb.iterate();
ByteBufferPool.Sized sizedPool = new ByteBufferPool.Sized(getServer().getByteBufferPool(), true, getBufferSize());
Content.Source source = Content.Source.from(sizedPool, _filePath);
Content.Sink sink = (last, bytebuffer, cb) -> getNextInterceptor().write(last, bytebuffer, cb);
Callback disposer = Callback.from(callback, this::dispose);
Content.copy(source, sink, disposer);
}
}
}
Loading

0 comments on commit e89d1e6

Please sign in to comment.