[QFJ-626] ResendRequest (silently) aborted when one of the message to resend was (sent and) stored with a bad checksum Created: 11/Aug/11  Updated: 02/Apr/15  Resolved: 06/Oct/14

Status: Closed
Project: QuickFIX/J
Component/s: Engine
Affects Version/s: 1.5.0
Fix Version/s: 1.6.0

Type: Bug Priority: Default
Reporter: Olivier Lourdais Assignee: Christoph John
Resolution: Fixed Votes: 0
Labels: QuickfixJ, encoding, session
Environment:

Ubuntu SMP (2.6.35-25-generic)

java version "1.6.0_24"
Java(TM) SE Runtime Environment (build 1.6.0_24-b07)
Java HotSpot(TM) Client VM (build 19.1-b02, mixed mode, sharing)

Attachments: File Session.patch    
Issue Links:
Relates
relates to QFJ-808 Prevent setting illegal double values... Closed

 Description   

When a bug of the application around QuickFIX/J causes the generation of a message containing non-ISO-8859-1 characters (e.g. a price set to NaN or Infinity will be formatted to a special character by java.text.DecimalFormat), the computed checksum is wrong (checksum is computed on Unicode characters, before they are converted to bytes, and non-ISO-8859-1 characters will be replaced by '?' during the chars to bytes conversion, making the checksum inconsistent).
The peer FIX application will detect the problem and skip the affected message, then it will send a ResendRequest.

To handle the ResendRequest, QuickFIX/J will retrieve asked messages in bytes version, will convert them to chars (still containing '?' instead of non-ISO-8859-1 characters), then parse them (and hopefully resend them).
When parsing the message, QuickFIX/J detects the invalid checksum and throws an InvalidMessage exception.
The problem is that this exception is not caught when iterating over the messages to parse, but in the next(Message message) method, as if the error was in the ResendRequest message.

The consequence is that following messages are never sent (and that no SequenceReset message is sent for the peer FIX application to skip the erroneous message).

 Comments   
Comment by Olivier Lourdais [ 11/Aug/11 ]

Attached patch fixes the bug, making InvalidMessage exception simply caught around message parsing.

Comment by Jason Quek [ 04/Sep/14 ]

Hi,

I would like to raise that we should throw an InvalidArgumentException if a NaN, Positive Infinity, Negative Infinity value is set on a double field rather than letting it fail when being sent over the session and ignoring the message. This could be done until multi-byte transmission is supported.

Regards,
Jason

Comment by Staffan Ulfberg [ 10/Sep/14 ]

I agree that raising an exception in the setters is the correct thing to do since there is no way to transmit a NaN or Infinity over FIX.

Just to point out, however, that adding multi-byte support would not help. The fact that Java formats NaN and infinity values using unicode characters is irrelevant for how to represent them on the wire when using FIX. FIX float fields consists of ASCII characters "-", "0" - "9" and ".", and I do not think it is desirable to make an extension to a basic FIX type in quickfixj that will not be supported by other FIX engines.

Basically I'm arguing that throwing an exception is not just a temporary fix until multi-byte support is implemented. I think it is the correct thing to do. (Another possibility would be to create a "FixFloat" class to model this FIX type that does not have an exact corresponding Java type, but that is indeed a larger project and might be unpractical for other reasons.)

Staffan

Comment by Christoph John [ 26/Sep/14 ]

Hi Staffan, so if I get you right, the following code should get placed in DoubleField.java??

    public void setValue(Double value) {
        if (Double.isInfinite(value) || Double.isNaN(value)) {
            throw new NumberFormatException("Tried to set NaN or infinite value.");
        }
        setObject(value);
    }

    public void setValue(double value) {
        if (Double.isInfinite(value) || Double.isNaN(value)) {
            throw new NumberFormatException("Tried to set NaN or infinite value.");
        }
        setObject(value);
    }
Comment by Staffan Ulfberg [ 26/Sep/14 ]

Yes, I think that would be great, but please add a check in the three constructors as well...

Staffan

Comment by Christoph John [ 29/Sep/14 ]

See QFJ-808.

Comment by Christoph John [ 06/Oct/14 ]

https://github.com/quickfix-j/quickfixj/commit/dbac498696c2425ea82576ef2b9163791a896ed2

Generated at Sat Nov 23 05:13:39 UTC 2024 using JIRA 7.5.2#75007-sha1:9f5725bb824792b3230a5d8716f0c13e296a3cae.