Skip to content

Commit

Permalink
Make Exception wrapper throw instead of returning -1/0
Browse files Browse the repository at this point in the history
This will hopefully fix issues like #4438.

PiperOrigin-RevId: 350393488
  • Loading branch information
sjudd authored and glide-copybara-robot committed Jan 6, 2021
1 parent 2b61482 commit 59ec98c
Show file tree
Hide file tree
Showing 5 changed files with 369 additions and 178 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import com.bumptech.glide.load.engine.Resource;
import com.bumptech.glide.load.engine.bitmap_recycle.ArrayPool;
import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
import com.bumptech.glide.util.ExceptionCatchingInputStream;
import com.bumptech.glide.util.ExceptionPassthroughInputStream;
import com.bumptech.glide.util.MarkEnforcingInputStream;
import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -49,8 +49,8 @@ public Resource<Bitmap> decode(
// Use to retrieve exceptions thrown while reading.
// TODO(#126): when the framework no longer returns partially decoded Bitmaps or provides a
// way to determine if a Bitmap is partially decoded, consider removing.
ExceptionCatchingInputStream exceptionStream =
ExceptionCatchingInputStream.obtain(bufferedStream);
ExceptionPassthroughInputStream exceptionStream =
ExceptionPassthroughInputStream.obtain(bufferedStream);

// Use to read data.
// Ensures that we can always reset after reading an image header so that we can still
Expand All @@ -74,11 +74,11 @@ public Resource<Bitmap> decode(
*/
static class UntrustedCallbacks implements Downsampler.DecodeCallbacks {
private final RecyclableBufferedInputStream bufferedStream;
private final ExceptionCatchingInputStream exceptionStream;
private final ExceptionPassthroughInputStream exceptionStream;

UntrustedCallbacks(
RecyclableBufferedInputStream bufferedStream,
ExceptionCatchingInputStream exceptionStream) {
ExceptionPassthroughInputStream exceptionStream) {
this.bufferedStream = bufferedStream;
this.exceptionStream = exceptionStream;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@
* android.graphics.BitmapFactory} can return partially decoded bitmaps.
*
* <p>See https://github.com/bumptech/glide/issues/126.
*
* @deprecated In some cases, callers may not handle getting 0 or -1 return values from methods,
* which can lead to infinite loops (see #4438). Use {@link ExceptionPassthroughInputStream}
* instead. This class will be deleted in a future version of Glide.
*/
@Deprecated
public class ExceptionCatchingInputStream extends InputStream {

private static final Queue<ExceptionCatchingInputStream> QUEUE = Util.createQueue(0);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package com.bumptech.glide.util;

import androidx.annotation.GuardedBy;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import java.io.IOException;
import java.io.InputStream;
import java.util.Queue;

/**
* An {@link java.io.InputStream} that catches, stores and rethrows {@link java.io.IOException}s
* during read and skip calls. This allows users of this API to handle the exception at a higher
* level if the exception is swallowed by some intermediate library. This class is a workaround for
* a framework issue where exceptions during reads while decoding bitmaps in {@link
* android.graphics.BitmapFactory} can return partially decoded bitmaps.
*
* <p>Unlike the deprecated {@link ExceptionCatchingInputStream}, this class will both store and
* re-throw any IOExceptions. Rethrowing works around bugs in wrapping streams that may not fully
* obey the stream contract. This is really only useful if some middle layer is going to catch the
* exception (like BitmapFactory) but we want to propagate the exception instead.
*
* <p>See https://github.com/bumptech/glide/issues/126 and #4438.
*/
public final class ExceptionPassthroughInputStream extends InputStream {

@GuardedBy("POOL")
private static final Queue<ExceptionPassthroughInputStream> POOL = Util.createQueue(0);

private InputStream wrapped;
private IOException exception;

@NonNull
public static ExceptionPassthroughInputStream obtain(@NonNull InputStream toWrap) {
ExceptionPassthroughInputStream result;
synchronized (POOL) {
result = POOL.poll();
}
if (result == null) {
result = new ExceptionPassthroughInputStream();
}
result.setInputStream(toWrap);
return result;
}

// Exposed for testing.
static void clearQueue() {
synchronized (POOL) {
while (!POOL.isEmpty()) {
POOL.remove();
}
}
}

ExceptionPassthroughInputStream() {
// Do nothing.
}

void setInputStream(@NonNull InputStream toWrap) {
wrapped = toWrap;
}

@Override
public int available() throws IOException {
return wrapped.available();
}

@Override
public void close() throws IOException {
wrapped.close();
}

@Override
public void mark(int readLimit) {
wrapped.mark(readLimit);
}

@Override
public boolean markSupported() {
return wrapped.markSupported();
}

@Override
public int read() throws IOException {
try {
return wrapped.read();
} catch (IOException e) {
exception = e;
throw e;
}
}

@Override
public int read(byte[] buffer) throws IOException {
try {
return wrapped.read(buffer);
} catch (IOException e) {
exception = e;
throw e;
}
}

@Override
public int read(byte[] buffer, int byteOffset, int byteCount) throws IOException {
try {
return wrapped.read(buffer, byteOffset, byteCount);
} catch (IOException e) {
exception = e;
throw e;
}
}

@Override
public synchronized void reset() throws IOException {
wrapped.reset();
}

@Override
public long skip(long byteCount) throws IOException {
try {
return wrapped.skip(byteCount);
} catch (IOException e) {
exception = e;
throw e;
}
}

@Nullable
public IOException getException() {
return exception;
}

public void release() {
exception = null;
wrapped = null;
synchronized (POOL) {
POOL.offer(this);
}
}
}

This file was deleted.

Loading

0 comments on commit 59ec98c

Please sign in to comment.