Re: Bug fix (not sure how to submit)

Lee Howard <[email protected]> Sun, 29 Aug 2010 22:48:56 -0700
Newsgroups gmane.comp.telephony.fax.hylafax.devel
Message-ID <[email protected]>
Patrice Fournier wrote:
> Brian,
>
> Brian Postow wrote:
>
>> We had a problem dealing with a buggy FOIP box that would get the T30 
>> frame, checked the CRC, found that it was correct, and then turned it 
>> into T38 packets and sent them to our computer. However there were 
>> two problems: 1) the FOIP box lost packets. and 2) T38 doesn't pass 
>> the CRC, it just says "yup, the CRC was correct on this end" and so, 
>> Hylafax got a short frame, which ends up with a broken TIFF file.
>>
>> We fixed this problem in faxd/Class1Recv.c++ by in additon to 
>> checking the CRC, checking the frame length:
>>
>> In faxd/Class1Recv.c++, in recvPageECMData line 1373
>>     if (frame.checkCRC())     becomes
>>         if (frame.checkCRC()  && frame.getLength() == frameSize+6)
>> This is in the 6.0.4 version... 
>
> The problem I have with this patch is that recvECMFrame() can return a
> valid frame with a length different to frameSize+6 (but this should 
> only be valid if it's the last frame). The following comment appears 
> in the code of that function:
>
> /*
>  * The HDLC frame was terminated early by a flag.  T.30 A.3.5 states that
>  * frame size cannot change during one page, and T.4 A.3.6.2 seems to 
> provide
>  * for padding in order to get that last frame on a block to always 
> line up
>  * on a byte and frame boundary.  However, the NOTE 2 there seemse to 
> give
>  * leniency to that requirement, and in fact many senders will send short
>  * frames on the last frame of a block.  So we run a couple of additional
>  * checks here (in addition to FCS checking) to limit the remote chance
>  * of FCS actually checking out on corrupt data (although that may be 
> very
>  * remote indeed).  We don't do these "trailing flag" tests on 
> normal-sized
>  * frames because we deliberately don't look for a trailing flag when we
>  * get enough data.
>  */ 

Indeed, Patrice is quite right here.  I have seen a rare sender send a 
block where the last frame in the block was "short" but where all of the 
image data was there (so they just didn't fill-out the last frame with 
padding).  So for those systems where the ECM frame CRC bytes are 
trustworthy requiring the frame length to be complete would actually 
become problematic in those cases.

I've thought on this problem over the last few months... in-particular 
I've tried to consider if there was any value in trying to get the Class 
1 ECM functions to perform the frame-length check on all frames except 
the last one (which gets to be a bit complicated since the frame 
checking is done on-the-fly instead of after the entire block is 
received).  But the problem with doing that (besides the needless 
introduction of a lot of operations for a lot of systems where the CRC 
bytes are trustworthy) is that it wouldn't fully satisfy the needs for 
those who need the frame-length checking in the first place because 
they'd be leaving the validity of the last frame up to chance.

So I've chosen to implement the requested frame-length check only on the 
condition that a new configuration setting, Class1ECMCheckFrameLength, 
is set to "true".  (It defaults to "false".)

For those users who enable this option, then, they will have problems 
receiving faxes from senders where the last frame of the block is 
short.  I don't see how this can be avoided, but fortunately it is a 
rare case, and I expect that it's significantly more-rare than the 
chances that the last frame on a block could be short due to data-loss 
as Brian has described.

Thanks,

Lee.


____________________ HylaFAX(tm) Developers Mailing List ____________________
  To subscribe/unsubscribe, click http://lists.hylafax.org/cgi-bin/lsg2.cgi
On UNIX: mail -s unsubscribe [email protected] < /dev/null