diff --git a/quickfixj-core/src/main/java/quickfix/FieldException.java b/quickfixj-core/src/main/java/quickfix/FieldException.java index 4e07e0f3a..73414872e 100644 --- a/quickfixj-core/src/main/java/quickfix/FieldException.java +++ b/quickfixj-core/src/main/java/quickfix/FieldException.java @@ -19,7 +19,7 @@ package quickfix; -public class FieldException extends RuntimeException { +public class FieldException extends RuntimeException implements HasFieldAndReason { private final int field; @@ -45,10 +45,12 @@ public boolean isFieldSpecified() { return field != -1; } + @Override public int getField() { return field; } + @Override public int getSessionRejectReason() { return sessionRejectReason; } diff --git a/quickfixj-core/src/main/java/quickfix/FieldNotFound.java b/quickfixj-core/src/main/java/quickfix/FieldNotFound.java index 1b08d3f10..63d8f2b3d 100644 --- a/quickfixj-core/src/main/java/quickfix/FieldNotFound.java +++ b/quickfixj-core/src/main/java/quickfix/FieldNotFound.java @@ -27,7 +27,7 @@ public class FieldNotFound extends Exception { public FieldNotFound(int field) { - super("Field [" + field + "] was not found in message."); + super("Field was not found in message, field=" + field); this.field = field; } diff --git a/quickfixj-core/src/main/java/quickfix/HasFieldAndReason.java b/quickfixj-core/src/main/java/quickfix/HasFieldAndReason.java new file mode 100644 index 000000000..7d30c60b7 --- /dev/null +++ b/quickfixj-core/src/main/java/quickfix/HasFieldAndReason.java @@ -0,0 +1,27 @@ +/******************************************************************************* + * Copyright (c) quickfixengine.org All rights reserved. + * + * This file is part of the QuickFIX FIX Engine + * + * This file may be distributed under the terms of the quickfixengine.org + * license as defined by quickfixengine.org and appearing in the file + * LICENSE included in the packaging of this file. + * + * This file is provided AS IS with NO WARRANTY OF ANY KIND, INCLUDING + * THE WARRANTY OF DESIGN, MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE. + * + * See http://www.quickfixengine.org/LICENSE for licensing information. + * + * Contact ask@quickfixengine.org if any conditions of this licensing + * are not clear to you. + ******************************************************************************/ + +package quickfix; + +interface HasFieldAndReason { + + int getField(); + int getSessionRejectReason(); + String getMessage(); +} diff --git a/quickfixj-core/src/main/java/quickfix/IncorrectDataFormat.java b/quickfixj-core/src/main/java/quickfix/IncorrectDataFormat.java index 5068196cf..b1ae253f4 100644 --- a/quickfixj-core/src/main/java/quickfix/IncorrectDataFormat.java +++ b/quickfixj-core/src/main/java/quickfix/IncorrectDataFormat.java @@ -19,19 +19,22 @@ package quickfix; +import quickfix.field.SessionRejectReason; + /** * Field has a badly formatted value. (From the C++ API documentation.) */ -public class IncorrectDataFormat extends Exception { - public final int field; - public final String data; +public class IncorrectDataFormat extends Exception implements HasFieldAndReason { + private final int field; + private final String data; + private final int sessionRejectReason; /** * @param field the tag number with the incorrect data * @param data the incorrect data */ public IncorrectDataFormat(final int field, final String data) { - this(field, data, "Field [" + field + "] contains badly formatted data."); + this(field, data, SessionRejectReasonText.getMessage(SessionRejectReason.INCORRECT_DATA_FORMAT_FOR_VALUE) + ", field=" + field); } /** @@ -51,10 +54,25 @@ public IncorrectDataFormat(final int field) { public IncorrectDataFormat(final String message) { this(0, null, message); } + + @Override + public int getSessionRejectReason() { + return sessionRejectReason; + } + @Override + public int getField() { + return field; + } + + public String getData() { + return data; + } + private IncorrectDataFormat(final int field, final String data, final String message) { super(message); this.field = field; this.data = data; + this.sessionRejectReason = SessionRejectReason.INCORRECT_DATA_FORMAT_FOR_VALUE; } } diff --git a/quickfixj-core/src/main/java/quickfix/IncorrectTagValue.java b/quickfixj-core/src/main/java/quickfix/IncorrectTagValue.java index 1d55ea7c4..1b1579e2a 100644 --- a/quickfixj-core/src/main/java/quickfix/IncorrectTagValue.java +++ b/quickfixj-core/src/main/java/quickfix/IncorrectTagValue.java @@ -19,24 +19,28 @@ package quickfix; +import quickfix.field.SessionRejectReason; + /** * An exception thrown when a tags value is not valid according to the data dictionary. */ -public class IncorrectTagValue extends Exception { +public class IncorrectTagValue extends Exception implements HasFieldAndReason { + + private String value; + private final int field; + private final int sessionRejectReason; public IncorrectTagValue(int field) { - super("Field [" + field + "] contains an incorrect tag value."); + super(SessionRejectReasonText.getMessage(SessionRejectReason.VALUE_IS_INCORRECT) + ", field=" + field); this.field = field; + this.sessionRejectReason = SessionRejectReason.VALUE_IS_INCORRECT; } public IncorrectTagValue(int field, String value) { super(); this.field = field; this.value = value; - } - - public IncorrectTagValue(String s) { - super(s); + this.sessionRejectReason = SessionRejectReason.VALUE_IS_INCORRECT; } @Override @@ -49,7 +53,14 @@ public String toString() { return str; } - public int field; + @Override + public int getField() { + return field; + } - public String value; + @Override + public int getSessionRejectReason() { + return sessionRejectReason; + } + } diff --git a/quickfixj-core/src/main/java/quickfix/Session.java b/quickfixj-core/src/main/java/quickfix/Session.java index 14d23395c..66c8d607c 100644 --- a/quickfixj-core/src/main/java/quickfix/Session.java +++ b/quickfixj-core/src/main/java/quickfix/Session.java @@ -1015,20 +1015,11 @@ private void next(Message message, boolean isProcessingQueuedMessages) throws Fi state.incrNextTargetMsgSeqNum(); break; } - } catch (final FieldException e) { + } catch (final FieldException | IncorrectDataFormat | IncorrectTagValue e) { if (logErrorAndDisconnectIfRequired(e, message)) { return; } - if (msgType.equals(MsgType.LOGON)) { - final String reason = SessionRejectReasonText.getMessage(e.getSessionRejectReason()); - final String errorMessage = "Invalid Logon message: " + (reason != null ? reason : "unspecific reason") - + " (field " + e.getField() + ")"; - generateLogout(errorMessage); - state.incrNextTargetMsgSeqNum(); - disconnect(errorMessage, true); - } else { - generateReject(message, e.getMessage(), e.getSessionRejectReason(), e.getField()); - } + handleExceptionAndRejectMessage(msgType, message, e); } catch (final FieldNotFound e) { if (logErrorAndDisconnectIfRequired(e, message)) { return; @@ -1045,15 +1036,11 @@ private void next(Message message, boolean isProcessingQueuedMessages) throws Fi generateReject(message, SessionRejectReason.REQUIRED_TAG_MISSING, e.field); } } - } catch (final IncorrectDataFormat e) { - if (logErrorAndDisconnectIfRequired(e, message)) { - return; - } - generateReject(message, SessionRejectReason.INCORRECT_DATA_FORMAT_FOR_VALUE, e.field); - } catch (final IncorrectTagValue e) { - getLog().onErrorEvent("Rejecting invalid message: " + e + ": " + message); - generateReject(message, SessionRejectReason.VALUE_IS_INCORRECT, e.field); } catch (final InvalidMessage e) { + /* InvalidMessage means a low-level error (e.g. checksum problem) and we should + ignore the message and let the problem correct itself (optimistic approach). + Target sequence number is not incremented, so it will trigger a ResendRequest + on the next message that is received. */ getLog().onErrorEvent("Skipping invalid message: " + e + ": " + message); if (resetOrDisconnectIfRequired(message)) { return; @@ -1135,9 +1122,28 @@ private void next(Message message, boolean isProcessingQueuedMessages) throws Fi } } + private void handleExceptionAndRejectMessage(final String msgType, final Message message, final HasFieldAndReason e) throws FieldNotFound, IOException { + if (MsgType.LOGON.equals(msgType)) { + logoutWithErrorMessage(e.getMessage()); + } else { + getLog().onErrorEvent("Rejecting invalid message: " + e + ": " + message); + generateReject(message, e.getMessage(), e.getSessionRejectReason(), e.getField()); + } + } + + private void logoutWithErrorMessage(final String reason) throws IOException { + final String errorMessage = "Invalid Logon message: " + (reason != null ? reason : "unspecific reason"); + generateLogout(errorMessage); + state.incrNextTargetMsgSeqNum(); + disconnect(errorMessage, true); + } + private boolean logErrorAndDisconnectIfRequired(final Exception e, Message message) { - getLog().onErrorEvent("Rejecting invalid message: " + e + ": " + message); - return resetOrDisconnectIfRequired(message); + final boolean resetOrDisconnectIfRequired = resetOrDisconnectIfRequired(message); + if (resetOrDisconnectIfRequired) { + getLog().onErrorEvent("Encountered invalid message: " + e + ": " + message); + } + return resetOrDisconnectIfRequired; } /** @@ -1600,7 +1606,7 @@ private void setRejectReason(Message reject, int field, String reason, } else { String rejectReason = reason; if (includeFieldInReason && !rejectReason.endsWith("" + field) ) { - rejectReason = rejectReason + " (" + field + ")"; + rejectReason = rejectReason + ", field=" + field; } reject.setString(Text.FIELD, rejectReason); } diff --git a/quickfixj-core/src/test/java/quickfix/ExceptionTest.java b/quickfixj-core/src/test/java/quickfix/ExceptionTest.java index 1f093e51a..7da22e93f 100644 --- a/quickfixj-core/src/test/java/quickfix/ExceptionTest.java +++ b/quickfixj-core/src/test/java/quickfix/ExceptionTest.java @@ -29,14 +29,13 @@ public void testDoNotSend() { public void testIncorrectDataFormat() { IncorrectDataFormat e = new IncorrectDataFormat(5, "test"); - assertEquals(5, e.field); - assertEquals("test", e.data); + assertEquals(5, e.getField()); + assertEquals("test", e.getData()); } public void testIncorrectTagValue() { new IncorrectTagValue(5); - IncorrectTagValue e = new IncorrectTagValue("test"); - e.field = 5; + IncorrectTagValue e = new IncorrectTagValue(5, "test"); } public void testRejectLogon() { diff --git a/quickfixj-core/src/test/java/quickfix/MessageCrackerTest.java b/quickfixj-core/src/test/java/quickfix/MessageCrackerTest.java index 169d9710b..2056517d7 100644 --- a/quickfixj-core/src/test/java/quickfix/MessageCrackerTest.java +++ b/quickfixj-core/src/test/java/quickfix/MessageCrackerTest.java @@ -91,7 +91,7 @@ public void testInvokerException3() throws Exception { MessageCracker cracker = new MessageCracker() { @Handler public void handle(quickfix.fixt11.Logon logon, SessionID sessionID) throws IncorrectTagValue { - throw new IncorrectTagValue("test"); + throw new IncorrectTagValue(0, "test"); } }; diff --git a/quickfixj-core/src/test/java/quickfix/RepeatingGroupTest.java b/quickfixj-core/src/test/java/quickfix/RepeatingGroupTest.java index e07031027..1132a307f 100644 --- a/quickfixj-core/src/test/java/quickfix/RepeatingGroupTest.java +++ b/quickfixj-core/src/test/java/quickfix/RepeatingGroupTest.java @@ -531,7 +531,7 @@ public void testInvalidEnumFieldInGroup() throws Exception { Assert.fail("No exception"); } catch (final IncorrectTagValue e) { // expected - assertEquals(385, e.field); + assertEquals(385, e.getField()); } } diff --git a/quickfixj-core/src/test/java/quickfix/SessionTest.java b/quickfixj-core/src/test/java/quickfix/SessionTest.java index ed578a0a1..56e376343 100644 --- a/quickfixj-core/src/test/java/quickfix/SessionTest.java +++ b/quickfixj-core/src/test/java/quickfix/SessionTest.java @@ -855,8 +855,8 @@ public void testSessionNotResetRightAfterLogonOnInitiator() throws Exception { } @Test - // QFJ-929 - public void testLogonIsAnsweredWithLogoutOnFieldException() throws Exception { + // QFJ-929/QFJ-933 + public void testLogonIsAnsweredWithLogoutOnException() throws Exception { final SessionID sessionID = new SessionID( FixVersions.BEGINSTRING_FIX44, "SENDER", "TARGET"); @@ -900,17 +900,32 @@ public void testLogonIsAnsweredWithLogoutOnFieldException() throws Exception { session.setResponder(responder); session.logon(); session.next(); - setUpHeader(session.getSessionID(), logonRequest, true, 2); - logonRequest.removeField(HeartBtInt.FIELD); + setUpHeader(session.getSessionID(), logonRequest, true, 3); + logonRequest.setString(EncryptMethod.FIELD, "A"); logonRequest.toString(); // calculate length and checksum session.next(logonRequest); - // session should not be logged on due to missing HeartBtInt + // session should not be logged on due to IncorrectDataFormat assertFalse(session.isLoggedOn()); assertEquals(3, application.lastToAdminMessage().getHeader().getInt(MsgSeqNum.FIELD)); - assertEquals(MsgType.LOGOUT, application.lastToAdminMessage().getHeader().getString(MsgType.FIELD)); + assertEquals(MsgType.LOGOUT, application.lastToAdminMessage().getHeader().getString(MsgType.FIELD)); assertEquals(4, session.getStore().getNextTargetMsgSeqNum()); assertEquals(4, session.getStore().getNextSenderMsgSeqNum()); + + session.setResponder(responder); + session.logon(); + session.next(); + setUpHeader(session.getSessionID(), logonRequest, true, 3); + logonRequest.setString(EncryptMethod.FIELD, "99"); + logonRequest.toString(); // calculate length and checksum + session.next(logonRequest); + // session should not be logged on due to IncorrectTagValue + assertFalse(session.isLoggedOn()); + + assertEquals(4, application.lastToAdminMessage().getHeader().getInt(MsgSeqNum.FIELD)); + assertEquals(MsgType.LOGOUT, application.lastToAdminMessage().getHeader().getString(MsgType.FIELD)); + assertEquals(5, session.getStore().getNextTargetMsgSeqNum()); + assertEquals(5, session.getStore().getNextSenderMsgSeqNum()); session.close(); } diff --git a/quickfixj-core/src/test/resources/quickfix/test/acceptance/definitions/server/fix40/14e_IncorrectEnumValue.def b/quickfixj-core/src/test/resources/quickfix/test/acceptance/definitions/server/fix40/14e_IncorrectEnumValue.def index 659275d63..886fc6335 100644 --- a/quickfixj-core/src/test/resources/quickfix/test/acceptance/definitions/server/fix40/14e_IncorrectEnumValue.def +++ b/quickfixj-core/src/test/resources/quickfix/test/acceptance/definitions/server/fix40/14e_IncorrectEnumValue.def @@ -13,7 +13,7 @@ E8=FIX.4.09=5635=A34=149=ISLD52=00000000-00:00:0056=TW98=0108=210=0 #New order message with incorrect enum value. Handling instructions (21) = 4 I8=FIX.4.035=D34=249=TW52=