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.

Sign up for GitHub

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

Jump to bottom

Issue #3924 - Remove Resource.writeTo(OutputStream, long, long) #4558

Merged
merged 1 commit into from
Feb 11, 2020
Merged

Issue #3924 - Remove Resource.writeTo(OutputStream, long, long) #4558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 33 additions & 7 deletions jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.util.Collection;
Expand All @@ -46,9 +47,11 @@
import org.eclipse.jetty.http.QuotedQualityCSV;
import org.eclipse.jetty.io.WriterOutputStream;
import org.eclipse.jetty.server.resource.HttpContentRangeWriter;
import org.eclipse.jetty.server.resource.InputStreamRangeWriter;
import org.eclipse.jetty.server.resource.RangeWriter;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.MultiPartOutputStream;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.log.Log;
Expand Down Expand Up @@ -669,18 +672,14 @@ protected boolean sendData(HttpServletRequest request,
if (include)
{
// write without headers
content.getResource().writeTo(out, 0, content_length);
writeContent(content, out, 0, content_length);
}
// else if we can't do a bypass write because of wrapping
else if (written || !(out instanceof HttpOutput))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing really to do with the changes but this line could be cleaned up, the value of !(out instanceof HttpOutput) is always false.

{
// write normally
putHeaders(response, content, written ? -1 : 0);
ByteBuffer buffer = content.getIndirectBuffer();
if (buffer != null)
BufferUtil.writeTo(buffer, out);
else
content.getResource().writeTo(out, 0, content_length);
writeContent(content, out, 0, content_length);
}
// else do a bypass write
else
Expand Down Expand Up @@ -753,7 +752,7 @@ public String toString()
response.addDateHeader(HttpHeader.DATE.asString(), System.currentTimeMillis());
response.setHeader(HttpHeader.CONTENT_RANGE.asString(),
singleSatisfiableRange.toHeaderRangeString(content_length));
content.getResource().writeTo(out, singleSatisfiableRange.getFirst(), singleLength);
writeContent(content, out, singleSatisfiableRange.getFirst(), singleLength);
return true;
}

Expand Down Expand Up @@ -815,6 +814,33 @@ public String toString()
return true;
}

private static void writeContent(HttpContent content, OutputStream out, long start, long contentLength) throws IOException
{
// Is the write for the whole content?
if (start == 0 && content.getResource().length() == contentLength)
{
// attempt efficient ByteBuffer based write for whole content
ByteBuffer buffer = content.getIndirectBuffer();
if (buffer != null)
{
BufferUtil.writeTo(buffer, out);
return;
}

try (InputStream input = content.getResource().getInputStream())
{
IO.copy(input, out);
return;
}
}

// Use a ranged writer
try (InputStreamRangeWriter rangeWriter = new InputStreamRangeWriter(() -> content.getInputStream()))
{
rangeWriter.writeTo(out, start, contentLength);
}
}

protected void putHeaders(HttpServletResponse response, HttpContent content, long contentLength)
{
if (response instanceof Response)
Expand Down
2 changes: 1 addition & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/resource/InputStreamRangeWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public interface InputStreamSupplier
private long pos;

/**
* Create InputStremRangeWriter
* Create InputStreamRangeWriter
*
* @param inputStreamSupplier Supplier of the InputStream. If the stream needs to be regenerated, such as the next
* requested range being before the current position, then the current InputStream is closed and a new one obtained
Expand Down
88 changes: 0 additions & 88 deletions jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,94 +594,6 @@ public void copyTo(File destination) throws IOException
}
}

/**
* @param outputStream the output stream to write to
* @param start First byte to write
* @param count Bytes to write or -1 for all of them.
* @throws IOException if unable to copy the Resource to the output
*/
@Override
public void writeTo(OutputStream outputStream, long start, long count)
throws IOException
{
long length = count;

if (count < 0)
{
length = Files.size(path) - start;
}

try (SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ))
{
ByteBuffer buffer = BufferUtil.allocate(IO.bufferSize);
skipTo(channel, buffer, start);

// copy from channel to output stream
long readTotal = 0;
while (readTotal < length)
{
BufferUtil.clearToFill(buffer);
int size = (int)Math.min(IO.bufferSize, length - readTotal);
buffer.limit(size);
int readLen = channel.read(buffer);
BufferUtil.flipToFlush(buffer, 0);
BufferUtil.writeTo(buffer, outputStream);
readTotal += readLen;
}
}
}

private void skipTo(SeekableByteChannel channel, ByteBuffer buffer, long skipTo) throws IOException
{
try
{
if (channel.position() != skipTo)
{
channel.position(skipTo);
}
}
catch (UnsupportedOperationException e)
{
final int NO_PROGRESS_LIMIT = 3;

if (skipTo > 0)
{
long pos = 0;
long readLen;
int noProgressLoopLimit = NO_PROGRESS_LIMIT;
// loop till we reach desired point, break out on lack of progress.
while (noProgressLoopLimit > 0 && pos < skipTo)
{
BufferUtil.clearToFill(buffer);
int len = (int)Math.min(IO.bufferSize, (skipTo - pos));
buffer.limit(len);
readLen = channel.read(buffer);
if (readLen == 0)
{
noProgressLoopLimit--;
}
else if (readLen > 0)
{
pos += readLen;
noProgressLoopLimit = NO_PROGRESS_LIMIT;
}
else
{
// negative values means the stream was closed or reached EOF
// either way, we've hit a state where we can no longer
// fulfill the requested range write.
throw new IOException("EOF reached before SeekableByteChannel skip destination");
}
}

if (noProgressLoopLimit <= 0)
{
throw new IOException("No progress made to reach SeekableByteChannel skip position " + skipTo);
}
}
}
}

@Override
public String toString()
{
Expand Down
36 changes: 15 additions & 21 deletions jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
import java.net.URI;
import java.net.URL;
import java.nio.channels.ReadableByteChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.text.DateFormat;
import java.util.ArrayList;
import java.util.Base64;
Expand Down Expand Up @@ -818,25 +820,6 @@ private static String deTag(String raw)
return StringUtil.sanitizeXmlString(raw);
}

/**
* @param out the output stream to write to
* @param start First byte to write
* @param count Bytes to write or -1 for all of them.
* @throws IOException if unable to copy the Resource to the output
*/
public void writeTo(OutputStream out, long start, long count)
throws IOException
{
try (InputStream in = getInputStream())
{
in.skip(start);
if (count < 0)
IO.copy(in, out);
else
IO.copy(in, out, count);
}
}

/**
* Copy the Resource to the new destination file.
* <p>
Expand All @@ -851,9 +834,20 @@ public void copyTo(File destination)
if (destination.exists())
throw new IllegalArgumentException(destination + " exists");

try (OutputStream out = new FileOutputStream(destination))
// attempt simple file copy
File src = getFile();
if (src != null)
{
Files.copy(src.toPath(), destination.toPath(),
StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
return;
}

// use old school stream based copy
try (InputStream in = getInputStream();
OutputStream out = new FileOutputStream(destination))
{
writeTo(out, 0, -1);
IO.copy(in, out);
}
}

Expand Down
26 changes: 0 additions & 26 deletions jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,32 +111,6 @@ public void testNonDefaultFileSystemGetFile() throws URISyntaxException, IOExcep
}
}

@Test
public void testNonDefaultFileSystemWriteTo() throws URISyntaxException, IOException
{
Path exampleJar = MavenTestingUtils.getTestResourcePathFile("example.jar");

URI uri = new URI("jar", exampleJar.toUri().toASCIIString(), null);

Map<String, Object> env = new HashMap<>();
env.put("multi-release", "runtime");

try (FileSystem zipfs = FileSystems.newFileSystem(uri, env))
{
Path manifestPath = zipfs.getPath("/META-INF/MANIFEST.MF");
assertThat(manifestPath, is(not(nullValue())));

PathResource resource = new PathResource(manifestPath);
try (ByteArrayOutputStream out = new ByteArrayOutputStream())
{
resource.writeTo(out, 2, 10);
String actual = new String(out.toByteArray(), UTF_8);
String expected = "nifest-Ver";
assertThat("writeTo(out, 2, 10)", actual, is(expected));
}
}
}

@Test
public void testDefaultFileSystemGetFile() throws Exception
{
Expand Down