From ec793c66ffab343ab32cb659e32c279be9279a90 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 27 Oct 2022 12:42:30 +0200 Subject: [PATCH] More cleanup and better error reporting --- .../syntax_check/OpenPgpMessageSyntax.java | 15 ++++++++----- .../syntax_check/PDA.java | 19 +++++++++++----- .../syntax_check/Syntax.java | 5 ++++- .../syntax_check/Transition.java | 22 ++++++++++++++++++- .../syntax_check/PDATest.java | 2 +- 5 files changed, 48 insertions(+), 15 deletions(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/OpenPgpMessageSyntax.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/OpenPgpMessageSyntax.java index 4c811e9f..c6de8765 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/OpenPgpMessageSyntax.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/OpenPgpMessageSyntax.java @@ -6,6 +6,9 @@ package org.pgpainless.decryption_verification.syntax_check; import org.pgpainless.exception.MalformedOpenPgpMessageException; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + /** * This class describes the syntax for OpenPGP messages as specified by rfc4880. * @@ -19,7 +22,7 @@ import org.pgpainless.exception.MalformedOpenPgpMessageException; public class OpenPgpMessageSyntax implements Syntax { @Override - public Transition transition(State from, InputSymbol input, StackSymbol stackItem) + public @Nonnull Transition transition(@Nonnull State from, @Nonnull InputSymbol input, @Nullable StackSymbol stackItem) throws MalformedOpenPgpMessageException { switch (from) { case OpenPgpMessage: @@ -37,7 +40,7 @@ public class OpenPgpMessageSyntax implements Syntax { throw new MalformedOpenPgpMessageException(from, input, stackItem); } - Transition fromOpenPgpMessage(InputSymbol input, StackSymbol stackItem) + Transition fromOpenPgpMessage(@Nonnull InputSymbol input, @Nullable StackSymbol stackItem) throws MalformedOpenPgpMessageException { if (stackItem != StackSymbol.msg) { throw new MalformedOpenPgpMessageException(State.OpenPgpMessage, input, stackItem); @@ -65,7 +68,7 @@ public class OpenPgpMessageSyntax implements Syntax { } } - Transition fromLiteralMessage(InputSymbol input, StackSymbol stackItem) + Transition fromLiteralMessage(@Nonnull InputSymbol input, @Nullable StackSymbol stackItem) throws MalformedOpenPgpMessageException { switch (input) { case Signature: @@ -84,7 +87,7 @@ public class OpenPgpMessageSyntax implements Syntax { throw new MalformedOpenPgpMessageException(State.LiteralMessage, input, stackItem); } - Transition fromCompressedMessage(InputSymbol input, StackSymbol stackItem) + Transition fromCompressedMessage(@Nonnull InputSymbol input, @Nullable StackSymbol stackItem) throws MalformedOpenPgpMessageException { switch (input) { case Signature: @@ -103,7 +106,7 @@ public class OpenPgpMessageSyntax implements Syntax { throw new MalformedOpenPgpMessageException(State.CompressedMessage, input, stackItem); } - Transition fromEncryptedMessage(InputSymbol input, StackSymbol stackItem) + Transition fromEncryptedMessage(@Nonnull InputSymbol input, @Nullable StackSymbol stackItem) throws MalformedOpenPgpMessageException { switch (input) { case Signature: @@ -122,7 +125,7 @@ public class OpenPgpMessageSyntax implements Syntax { throw new MalformedOpenPgpMessageException(State.EncryptedMessage, input, stackItem); } - Transition fromValid(InputSymbol input, StackSymbol stackItem) + Transition fromValid(@Nonnull InputSymbol input, @Nullable StackSymbol stackItem) throws MalformedOpenPgpMessageException { // There is no applicable transition rule out of Valid throw new MalformedOpenPgpMessageException(State.Valid, input, stackItem); diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/PDA.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/PDA.java index 68a5e2c4..ed9175fd 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/PDA.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/PDA.java @@ -20,10 +20,13 @@ public class PDA { private static final Logger LOGGER = LoggerFactory.getLogger(PDA.class); + // right now we implement what rfc4880 specifies. + // TODO: Consider implementing what we proposed here: + // https://mailarchive.ietf.org/arch/msg/openpgp/uepOF6XpSegMO4c59tt9e5H1i4g/ + private final Syntax syntax = new OpenPgpMessageSyntax(); private final Stack stack = new Stack<>(); - private final List inputs = new ArrayList<>(); // keep track of inputs for debugging / error reporting + private final List inputs = new ArrayList<>(); // Track inputs for debugging / error reporting private State state; - private Syntax syntax = new OpenPgpMessageSyntax(); public PDA() { state = State.OpenPgpMessage; @@ -32,16 +35,20 @@ public class PDA { } public void next(InputSymbol input) throws MalformedOpenPgpMessageException { + StackSymbol stackSymbol = popStack(); try { - Transition transition = syntax.transition(state, input, popStack()); - inputs.add(input); + Transition transition = syntax.transition(state, input, stackSymbol); state = transition.getNewState(); for (StackSymbol item : transition.getPushedItems()) { pushStack(item); } + inputs.add(input); } catch (MalformedOpenPgpMessageException e) { - MalformedOpenPgpMessageException wrapped = new MalformedOpenPgpMessageException("Malformed message: After reading stream " + Arrays.toString(inputs.toArray()) + - ", token '" + input + "' is unexpected and illegal.", e); + MalformedOpenPgpMessageException wrapped = new MalformedOpenPgpMessageException( + "Malformed message: After reading stream " + Arrays.toString(inputs.toArray()) + + ", token '" + input + "' is not allowed." + + "\nNo transition from state '" + state + "' with stack " + Arrays.toString(stack.toArray()) + + (stackSymbol != null ? "||'" + stackSymbol + "'." : "."), e); LOGGER.debug("Invalid input '" + input + "'", wrapped); throw wrapped; } diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/Syntax.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/Syntax.java index 63d63fed..2f3d0a57 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/Syntax.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/Syntax.java @@ -6,6 +6,9 @@ package org.pgpainless.decryption_verification.syntax_check; import org.pgpainless.exception.MalformedOpenPgpMessageException; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + /** * This interface can be used to define a custom syntax for the {@link PDA}. */ @@ -25,6 +28,6 @@ public interface Syntax { * @return applicable transition rule containing the new state and pushed stack symbols * @throws MalformedOpenPgpMessageException if there is no applicable transition rule (the input symbol is illegal) */ - Transition transition(State from, InputSymbol input, StackSymbol stackItem) + @Nonnull Transition transition(@Nonnull State from, @Nonnull InputSymbol input, @Nullable StackSymbol stackItem) throws MalformedOpenPgpMessageException; } diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/Transition.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/Transition.java index a0e58cf0..ab0db5ef 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/Transition.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/syntax_check/Transition.java @@ -4,24 +4,44 @@ package org.pgpainless.decryption_verification.syntax_check; +import javax.annotation.Nonnull; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +/** + * Result of applying a transition rule. + * Transition rules can be described by implementing the {@link Syntax} interface. + */ public class Transition { private final List pushedItems = new ArrayList<>(); private final State newState; - public Transition(State newState, StackSymbol... pushedItems) { + public Transition(@Nonnull State newState, @Nonnull StackSymbol... pushedItems) { this.newState = newState; this.pushedItems.addAll(Arrays.asList(pushedItems)); } + /** + * Return the new {@link State} that is reached by applying the transition. + * + * @return new state + */ + @Nonnull public State getNewState() { return newState; } + /** + * Return a list of {@link StackSymbol StackSymbols} that are pushed onto the stack + * by applying the transition. + * The list contains items in the order in which they are pushed onto the stack. + * The list may be empty. + * + * @return list of items to be pushed onto the stack + */ + @Nonnull public List getPushedItems() { return new ArrayList<>(pushedItems); } diff --git a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/syntax_check/PDATest.java b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/syntax_check/PDATest.java index 2b66eee0..d0486a3e 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/syntax_check/PDATest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/syntax_check/PDATest.java @@ -97,7 +97,7 @@ public class PDATest { } @Test - public void testEncryptedMessageWithAppendedStandalongSigIsNotValid() { + public void testEncryptedMessageWithAppendedStandaloneSigIsNotValid() { PDA check = new PDA(); check.next(InputSymbol.EncryptedData); assertThrows(MalformedOpenPgpMessageException.class,