wbrawner/thunderbird-android
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1931 from k9mail/GH-1919_fix_ImapResponseParser_bug

Browse source 
Properly handle exceptions thrown by ImapResponseCallback
This commit is contained in:
cketti 2017-01-03 17:49:45 +01:00 committed by GitHub
commit a56f12f1be
4 changed files with 128 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
k9mail-library/src/main/java/com/fsck/k9/mail/filter/FixedLengthInputStream.java
View file

@ -69,4 +69,10 @@ public class FixedLengthInputStream extends InputStream {
public String toString() {
return String.format(Locale.US, "FixedLengthInputStream(in=%s, length=%d)", mIn.toString(), mLength);
}
public void skipRemaining() throws IOException {
while (available() > 0) {
skip(available());
}
}
}

26
k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParser.java
View file

@ -45,7 +45,7 @@ class ImapResponseParser {
}
if (exception != null) {
throw new RuntimeException("readResponse(): Exception in callback method", exception);
throw new ImapResponseParserException("readResponse(): Exception in callback method", exception);
}
return response;
@ -354,26 +354,32 @@ class ImapResponseParser {
if (response.getCallback() != null) {
FixedLengthInputStream fixed = new FixedLengthInputStream(inputStream, size);
Exception callbackException = null;
Object result = null;
try {
result = response.getCallback().foundLiteral(response, fixed);
} catch (IOException e) {
// Pass IOExceptions through
throw e;
} catch (Exception e) {
// Catch everything else and save it for later.
exception = e;
callbackException = e;
}
// Check if only some of the literal data was read
int available = fixed.available();
if (available > 0 && available != size) {
// If so, skip the rest
while (fixed.available() > 0) {
fixed.skip(fixed.available());
boolean someDataWasRead = fixed.available() != size;
if (someDataWasRead) {
if (result == null && callbackException == null) {
throw new AssertionError("Callback consumed some data but returned no result");
}
fixed.skipRemaining();
}
if (callbackException != null) {
if (exception == null) {
exception = callbackException;
}
return "EXCEPTION";
}
if (result != null) {
return result;
}

8
k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParserException.java Normal file
View file

@ -0,0 +1,8 @@
package com.fsck.k9.mail.store.imap;
public class ImapResponseParserException extends RuntimeException {
public ImapResponseParserException(String message, Throwable cause) {
super(message, cause);
}
}

117
k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapResponseParserTest.java
View file

@ -23,6 +23,8 @@ import static org.junit.Assert.fail;
@RunWith(RobolectricTestRunner.class)
@Config(manifest = Config.NONE, sdk = 21)
public class ImapResponseParserTest {
private PeekableInputStream peekableInputStream;
@Test
public void testSimpleOkResponse() throws IOException {
@ -231,7 +233,7 @@ public class ImapResponseParserTest {
@Test
public void testParseLiteralWithConsumingCallbackReturningNull() throws Exception {
ImapResponseParser parser = createParser("* {4}\r\ntest\r\n");
TestImapResponseCallback callback = new TestImapResponseCallback(4, "cheeseburger");
TestImapResponseCallback callback = TestImapResponseCallback.readBytesAndReturn(4, "cheeseburger");
ImapResponse response = parser.readResponse(callback);
@ -242,37 +244,88 @@ public class ImapResponseParserTest {
@Test
public void testParseLiteralWithNonConsumingCallbackReturningNull() throws Exception {
ImapResponseParser parser = createParser("* {4}\r\ntest\r\n");
TestImapResponseCallback callback = new TestImapResponseCallback(0, null);
TestImapResponseCallback callback = TestImapResponseCallback.readBytesAndReturn(0, null);
ImapResponse response = parser.readResponse(callback);
assertEquals(1, response.size());
assertEquals("test", response.getString(0));
assertTrue(callback.foundLiteralCalled);
assertAllInputConsumed();
}
@Test
public void readResponse_withPartlyConsumingCallbackReturningNull_shouldThrow() throws Exception {
ImapResponseParser parser = createParser("* {4}\r\ntest\r\n");
TestImapResponseCallback callback = TestImapResponseCallback.readBytesAndReturn(2, null);
try {
parser.readResponse(callback);
fail();
} catch (AssertionError e) {
assertEquals("Callback consumed some data but returned no result", e.getMessage());
}
}
@Test
public void readResponse_withPartlyConsumingCallbackThatThrows_shouldReadAllDataAndThrow() throws Exception {
ImapResponseParser parser = createParser("* {4}\r\ntest\r\n");
TestImapResponseCallback callback = TestImapResponseCallback.readBytesAndThrow(2);
try {
parser.readResponse(callback);
fail();
} catch (ImapResponseParserException e) {
assertEquals("readResponse(): Exception in callback method", e.getMessage());
assertEquals(ImapResponseParserTestException.class, e.getCause().getClass());
}
assertAllInputConsumed();
}
@Test
public void readResponse_withCallbackThatThrowsRepeatedly_shouldConsumeAllInputAndThrowFirstException()
throws Exception {
ImapResponseParser parser = createParser("* {3}\r\none {3}\r\ntwo\r\n");
TestImapResponseCallback callback = TestImapResponseCallback.readBytesAndThrow(3);
try {
parser.readResponse(callback);
fail();
} catch (ImapResponseParserException e) {
assertEquals("readResponse(): Exception in callback method", e.getMessage());
assertEquals(ImapResponseParserTestException.class, e.getCause().getClass());
assertEquals(0, ((ImapResponseParserTestException) e.getCause()).instanceNumber);
}
assertAllInputConsumed();
}
@Test
public void testParseLiteralWithIncompleteConsumingCallbackReturningString() throws Exception {
ImapResponseParser parser = createParser("* {4}\r\ntest\r\n");
TestImapResponseCallback callback = new TestImapResponseCallback(2, "ninja");
TestImapResponseCallback callback = TestImapResponseCallback.readBytesAndReturn(2, "ninja");
ImapResponse response = parser.readResponse(callback);
assertEquals(1, response.size());
assertEquals("ninja", response.getString(0));
assertAllInputConsumed();
}
@Test(expected = RuntimeException.class)
@Test
public void testParseLiteralWithThrowingCallback() throws Exception {
ImapResponseParser parser = createParser("* {4}\r\ntest\r\n");
ImapResponseCallback callback = new ImapResponseCallback() {
@Override
public Object foundLiteral(ImapResponse response, FixedLengthInputStream literal) throws Exception {
throw new RuntimeException();
}
};
ImapResponseCallback callback = TestImapResponseCallback.readBytesAndThrow(0);
parser.readResponse(callback);
try {
parser.readResponse(callback);
fail();
} catch (ImapResponseParserException e) {
assertEquals("readResponse(): Exception in callback method", e.getMessage());
}
assertAllInputConsumed();
}
@Test(expected = IOException.class)
@ -415,19 +468,34 @@ public class ImapResponseParserTest {
private ImapResponseParser createParser(String response) {
ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(response.getBytes());
PeekableInputStream peekableInputStream = new PeekableInputStream(byteArrayInputStream);
peekableInputStream = new PeekableInputStream(byteArrayInputStream);
return new ImapResponseParser(peekableInputStream);
}
private void assertAllInputConsumed() throws IOException {
assertEquals(0, peekableInputStream.available());
}
private class TestImapResponseCallback implements ImapResponseCallback {
static class TestImapResponseCallback implements ImapResponseCallback {
private final int readNumberOfBytes;
private final Object returnValue;
private final boolean throwException;
private int exceptionCount = 0;
public boolean foundLiteralCalled = false;
TestImapResponseCallback(int readNumberOfBytes, Object returnValue) {
public static TestImapResponseCallback readBytesAndReturn(int readNumberOfBytes, Object returnValue) {
return new TestImapResponseCallback(readNumberOfBytes, returnValue, false);
}
public static TestImapResponseCallback readBytesAndThrow(int readNumberOfBytes) {
return new TestImapResponseCallback(readNumberOfBytes, null, true);
}
private TestImapResponseCallback(int readNumberOfBytes, Object returnValue, boolean throwException) {
this.readNumberOfBytes = readNumberOfBytes;
this.returnValue = returnValue;
this.throwException = throwException;
}
@Override
@ -435,17 +503,28 @@ public class ImapResponseParserTest {
foundLiteralCalled = true;
int skipBytes = readNumberOfBytes;
long skippedBytes;
do {
skippedBytes = literal.skip(skipBytes);
while (skipBytes > 0) {
long skippedBytes = literal.skip(skipBytes);
skipBytes -= skippedBytes;
} while (skippedBytes > 0);
}
if (throwException) {
throw new ImapResponseParserTestException(exceptionCount++);
}
return returnValue;
}
}
private class TestUntaggedHandler implements UntaggedHandler {
static class ImapResponseParserTestException extends RuntimeException {
public final int instanceNumber;
public ImapResponseParserTestException(int instanceNumber) {
this.instanceNumber = instanceNumber;
}
}
static class TestUntaggedHandler implements UntaggedHandler {
public final List<ImapResponse> responses = new ArrayList<ImapResponse>();
@Override