Skip to content

Commit 59ec98c

Browse files
sjuddglide-copybara-robot
sjudd
authored and
glide-copybara-robot
committedJan 6, 2021
Make Exception wrapper throw instead of returning -1/0
This will hopefully fix issues like #4438. PiperOrigin-RevId: 350393488
·
v5.0.0-rc01compose-alpha6
1 parent 2b61482 commit 59ec98c

File tree

5 files changed

+369
-178
lines changed

5 files changed

+369
-178
lines changed
 

‎library/src/main/java/com/bumptech/glide/load/resource/bitmap/StreamBitmapDecoder.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import com.bumptech.glide.load.engine.Resource;
88
import com.bumptech.glide.load.engine.bitmap_recycle.ArrayPool;
99
import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
10-
import com.bumptech.glide.util.ExceptionCatchingInputStream;
10+
import com.bumptech.glide.util.ExceptionPassthroughInputStream;
1111
import com.bumptech.glide.util.MarkEnforcingInputStream;
1212
import java.io.IOException;
1313
import java.io.InputStream;
@@ -49,8 +49,8 @@ public Resource<Bitmap> decode(
4949
// Use to retrieve exceptions thrown while reading.
5050
// TODO(#126): when the framework no longer returns partially decoded Bitmaps or provides a
5151
// way to determine if a Bitmap is partially decoded, consider removing.
52-
ExceptionCatchingInputStream exceptionStream =
53-
ExceptionCatchingInputStream.obtain(bufferedStream);
52+
ExceptionPassthroughInputStream exceptionStream =
53+
ExceptionPassthroughInputStream.obtain(bufferedStream);
5454

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

7979
UntrustedCallbacks(
8080
RecyclableBufferedInputStream bufferedStream,
81-
ExceptionCatchingInputStream exceptionStream) {
81+
ExceptionPassthroughInputStream exceptionStream) {
8282
this.bufferedStream = bufferedStream;
8383
this.exceptionStream = exceptionStream;
8484
}

‎library/src/main/java/com/bumptech/glide/util/ExceptionCatchingInputStream.java

+5
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@
1313
* android.graphics.BitmapFactory} can return partially decoded bitmaps.
1414
*
1515
* <p>See https://github.com/bumptech/glide/issues/126.
16+
*
17+
* @deprecated In some cases, callers may not handle getting 0 or -1 return values from methods,
18+
* which can lead to infinite loops (see #4438). Use {@link ExceptionPassthroughInputStream}
19+
* instead. This class will be deleted in a future version of Glide.
1620
*/
21+
@Deprecated
1722
public class ExceptionCatchingInputStream extends InputStream {
1823

1924
private static final Queue<ExceptionCatchingInputStream> QUEUE = Util.createQueue(0);

‎library/src/main/java/com/bumptech/glide/util/ExceptionPassthroughInputStream.java

+139
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package com.bumptech.glide.util;
2+
3+
import androidx.annotation.GuardedBy;
4+
import androidx.annotation.NonNull;
5+
import androidx.annotation.Nullable;
6+
import java.io.IOException;
7+
import java.io.InputStream;
8+
import java.util.Queue;
9+
10+
/**
11+
* An {@link java.io.InputStream} that catches, stores and rethrows {@link java.io.IOException}s
12+
* during read and skip calls. This allows users of this API to handle the exception at a higher
13+
* level if the exception is swallowed by some intermediate library. This class is a workaround for
14+
* a framework issue where exceptions during reads while decoding bitmaps in {@link
15+
* android.graphics.BitmapFactory} can return partially decoded bitmaps.
16+
*
17+
* <p>Unlike the deprecated {@link ExceptionCatchingInputStream}, this class will both store and
18+
* re-throw any IOExceptions. Rethrowing works around bugs in wrapping streams that may not fully
19+
* obey the stream contract. This is really only useful if some middle layer is going to catch the
20+
* exception (like BitmapFactory) but we want to propagate the exception instead.
21+
*
22+
* <p>See https://github.com/bumptech/glide/issues/126 and #4438.
23+
*/
24+
public final class ExceptionPassthroughInputStream extends InputStream {
25+
26+
@GuardedBy("POOL")
27+
private static final Queue<ExceptionPassthroughInputStream> POOL = Util.createQueue(0);
28+
29+
private InputStream wrapped;
30+
private IOException exception;
31+
32+
@NonNull
33+
public static ExceptionPassthroughInputStream obtain(@NonNull InputStream toWrap) {
34+
ExceptionPassthroughInputStream result;
35+
synchronized (POOL) {
36+
result = POOL.poll();
37+
}
38+
if (result == null) {
39+
result = new ExceptionPassthroughInputStream();
40+
}
41+
result.setInputStream(toWrap);
42+
return result;
43+
}
44+
45+
// Exposed for testing.
46+
static void clearQueue() {
47+
synchronized (POOL) {
48+
while (!POOL.isEmpty()) {
49+
POOL.remove();
50+
}
51+
}
52+
}
53+
54+
ExceptionPassthroughInputStream() {
55+
// Do nothing.
56+
}
57+
58+
void setInputStream(@NonNull InputStream toWrap) {
59+
wrapped = toWrap;
60+
}
61+
62+
@Override
63+
public int available() throws IOException {
64+
return wrapped.available();
65+
}
66+
67+
@Override
68+
public void close() throws IOException {
69+
wrapped.close();
70+
}
71+
72+
@Override
73+
public void mark(int readLimit) {
74+
wrapped.mark(readLimit);
75+
}
76+
77+
@Override
78+
public boolean markSupported() {
79+
return wrapped.markSupported();
80+
}
81+
82+
@Override
83+
public int read() throws IOException {
84+
try {
85+
return wrapped.read();
86+
} catch (IOException e) {
87+
exception = e;
88+
throw e;
89+
}
90+
}
91+
92+
@Override
93+
public int read(byte[] buffer) throws IOException {
94+
try {
95+
return wrapped.read(buffer);
96+
} catch (IOException e) {
97+
exception = e;
98+
throw e;
99+
}
100+
}
101+
102+
@Override
103+
public int read(byte[] buffer, int byteOffset, int byteCount) throws IOException {
104+
try {
105+
return wrapped.read(buffer, byteOffset, byteCount);
106+
} catch (IOException e) {
107+
exception = e;
108+
throw e;
109+
}
110+
}
111+
112+
@Override
113+
public synchronized void reset() throws IOException {
114+
wrapped.reset();
115+
}
116+
117+
@Override
118+
public long skip(long byteCount) throws IOException {
119+
try {
120+
return wrapped.skip(byteCount);
121+
} catch (IOException e) {
122+
exception = e;
123+
throw e;
124+
}
125+
}
126+
127+
@Nullable
128+
public IOException getException() {
129+
return exception;
130+
}
131+
132+
public void release() {
133+
exception = null;
134+
wrapped = null;
135+
synchronized (POOL) {
136+
POOL.offer(this);
137+
}
138+
}
139+
}

‎library/test/src/test/java/com/bumptech/glide/util/ExceptionCatchingInputStreamTest.java

-173
This file was deleted.

‎library/test/src/test/java/com/bumptech/glide/util/ExceptionPassthroughInputStreamTest.java

+220
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
package com.bumptech.glide.util;
2+
3+
import static com.google.common.truth.Truth.assertThat;
4+
import static org.junit.Assert.assertEquals;
5+
import static org.junit.Assert.assertNull;
6+
import static org.junit.Assert.assertThrows;
7+
import static org.junit.Assert.assertTrue;
8+
9+
import java.io.ByteArrayInputStream;
10+
import java.io.IOException;
11+
import java.io.InputStream;
12+
import java.net.SocketTimeoutException;
13+
import java.util.concurrent.atomic.AtomicBoolean;
14+
import org.junit.After;
15+
import org.junit.Before;
16+
import org.junit.Test;
17+
import org.junit.function.ThrowingRunnable;
18+
import org.junit.runner.RunWith;
19+
import org.junit.runners.JUnit4;
20+
21+
@RunWith(JUnit4.class)
22+
public class ExceptionPassthroughInputStreamTest {
23+
24+
private final InputStream validInputStream =
25+
new ByteArrayInputStream(
26+
new byte[] {
27+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
28+
});
29+
private final InputStream throwingInputStream = new ExceptionThrowingInputStream();
30+
private ExceptionPassthroughInputStream validWrapper;
31+
private ExceptionPassthroughInputStream throwingWrapper;
32+
33+
@Before
34+
public void setUp() throws Exception {
35+
validWrapper = new ExceptionPassthroughInputStream();
36+
validWrapper.setInputStream(validInputStream);
37+
throwingWrapper = new ExceptionPassthroughInputStream();
38+
throwingWrapper.setInputStream(throwingInputStream);
39+
}
40+
41+
@After
42+
public void tearDown() {
43+
ExceptionPassthroughInputStream.clearQueue();
44+
}
45+
46+
@Test
47+
public void testReturnsWrappedAvailable() throws IOException {
48+
assertEquals(validInputStream.available(), validWrapper.available());
49+
}
50+
51+
@Test
52+
public void testCallsCloseOnWrapped() throws IOException {
53+
ExceptionPassthroughInputStream wrapper = new ExceptionPassthroughInputStream();
54+
final AtomicBoolean isClosed = new AtomicBoolean();
55+
wrapper.setInputStream(
56+
new InputStream() {
57+
@Override
58+
public int read() {
59+
return 0;
60+
}
61+
62+
@Override
63+
public void close() throws IOException {
64+
super.close();
65+
isClosed.set(true);
66+
}
67+
});
68+
wrapper.close();
69+
assertThat(isClosed.get()).isTrue();
70+
}
71+
72+
@Test
73+
public void testCallsMarkOnWrapped() throws IOException {
74+
int toMark = 5;
75+
validWrapper.mark(toMark);
76+
assertThat(validWrapper.read(new byte[5], 0, 5)).isEqualTo(5);
77+
validInputStream.reset();
78+
assertThat(validInputStream.read()).isEqualTo(0);
79+
}
80+
81+
@Test
82+
public void testReturnsWrappedMarkSupported() {
83+
assertTrue(validWrapper.markSupported());
84+
}
85+
86+
@Test
87+
public void testCallsReadByteArrayOnWrapped() throws IOException {
88+
byte[] buffer = new byte[8];
89+
assertEquals(buffer.length, validWrapper.read(buffer));
90+
}
91+
92+
@Test
93+
public void testCallsReadArrayWithOffsetAndCountOnWrapped() throws IOException {
94+
int offset = 1;
95+
int count = 4;
96+
byte[] buffer = new byte[5];
97+
98+
assertEquals(count, validWrapper.read(buffer, offset, count));
99+
}
100+
101+
@Test
102+
public void testCallsReadOnWrapped() throws IOException {
103+
assertEquals(0, validWrapper.read());
104+
assertEquals(1, validWrapper.read());
105+
assertEquals(2, validWrapper.read());
106+
}
107+
108+
@Test
109+
public void testCallsResetOnWrapped() throws IOException {
110+
validWrapper.mark(5);
111+
assertThat(validWrapper.read()).isEqualTo(0);
112+
assertThat(validWrapper.read()).isEqualTo(1);
113+
validWrapper.reset();
114+
assertThat(validWrapper.read()).isEqualTo(0);
115+
}
116+
117+
@Test
118+
public void testCallsSkipOnWrapped() throws IOException {
119+
int toSkip = 5;
120+
assertThat(validWrapper.skip(toSkip)).isEqualTo(toSkip);
121+
assertThat(validWrapper.read()).isEqualTo(5);
122+
}
123+
124+
@Test
125+
public void testCatchesExceptionOnRead() {
126+
SocketTimeoutException expected =
127+
assertThrows(
128+
SocketTimeoutException.class,
129+
new ThrowingRunnable() {
130+
@Override
131+
public void run() throws Throwable {
132+
throwingWrapper.read();
133+
}
134+
});
135+
assertEquals(expected, throwingWrapper.getException());
136+
}
137+
138+
@Test
139+
public void testCatchesExceptionOnReadBuffer() {
140+
SocketTimeoutException exception =
141+
assertThrows(
142+
SocketTimeoutException.class,
143+
new ThrowingRunnable() {
144+
@Override
145+
public void run() throws Throwable {
146+
throwingWrapper.read(new byte[1]);
147+
}
148+
});
149+
assertEquals(exception, throwingWrapper.getException());
150+
}
151+
152+
@Test
153+
public void testCatchesExceptionOnReadBufferWithOffsetAndCount() {
154+
SocketTimeoutException exception =
155+
assertThrows(
156+
SocketTimeoutException.class,
157+
new ThrowingRunnable() {
158+
@Override
159+
public void run() throws Throwable {
160+
throwingWrapper.read(new byte[2], 1, 1);
161+
}
162+
});
163+
assertEquals(exception, throwingWrapper.getException());
164+
}
165+
166+
@Test
167+
public void testCatchesExceptionOnSkip() {
168+
SocketTimeoutException exception =
169+
assertThrows(
170+
SocketTimeoutException.class,
171+
new ThrowingRunnable() {
172+
@Override
173+
public void run() throws Throwable {
174+
throwingWrapper.skip(100);
175+
}
176+
});
177+
assertEquals(exception, throwingWrapper.getException());
178+
}
179+
180+
@Test
181+
public void testExceptionIsNotSetInitially() {
182+
assertNull(validWrapper.getException());
183+
}
184+
185+
@SuppressWarnings("ResultOfMethodCallIgnored")
186+
@Test
187+
public void testResetsExceptionToNullOnRelease() {
188+
assertThrows(
189+
SocketTimeoutException.class,
190+
new ThrowingRunnable() {
191+
@Override
192+
public void run() throws Throwable {
193+
throwingWrapper.read();
194+
}
195+
});
196+
throwingWrapper.release();
197+
assertNull(validWrapper.getException());
198+
}
199+
200+
@Test
201+
public void testCanReleaseAnObtainFromPool() {
202+
validWrapper.release();
203+
InputStream fromPool = ExceptionPassthroughInputStream.obtain(validInputStream);
204+
assertEquals(validWrapper, fromPool);
205+
}
206+
207+
@Test
208+
public void testCanObtainNewStreamFromPool() throws IOException {
209+
InputStream fromPool = ExceptionPassthroughInputStream.obtain(validInputStream);
210+
int read = fromPool.read();
211+
assertEquals(0, read);
212+
}
213+
214+
private static final class ExceptionThrowingInputStream extends InputStream {
215+
@Override
216+
public int read() throws IOException {
217+
throw new SocketTimeoutException();
218+
}
219+
}
220+
}

0 commit comments

Comments
 (0)
Please sign in to comment.