diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/filter/FixedLengthInputStream.java b/k9mail-library/src/main/java/com/fsck/k9/mail/filter/FixedLengthInputStream.java index 1125eef7a..9535822e0 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/filter/FixedLengthInputStream.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/filter/FixedLengthInputStream.java @@ -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()); + } + } } diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParser.java b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParser.java index 69243dbe1..71e422fd6 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParser.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParser.java @@ -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; } diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParserException.java b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParserException.java new file mode 100644 index 000000000..0238d9d5b --- /dev/null +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParserException.java @@ -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); + } +} diff --git a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapResponseParserTest.java b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapResponseParserTest.java index a1f748599..a2f5484cf 100644 --- a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapResponseParserTest.java +++ b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapResponseParserTest.java @@ -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 responses = new ArrayList(); @Override