[jcifs] Resyncing? Out of Phase

[Sebastian Sickelmann]

Am 11.08.2011 07:07, schrieb Michael B Allen:
> On Wed, Aug 10, 2011 at 4:21 PM, Sebastian Sickelmann
> <sebastian.sickelmann at gmx.de>  wrote:
>> Hi,
>>
>> In SMBTransport there is the following code fragment
>>                                         /* out of phase maybe? */
>>                           /* inch forward 1 byte and try again */
>>             for (int i = 0; i<  35; i++) {
>>                 sbuf[i] = sbuf[i + 1];
>>             }
>>             int b;
>>             if ((b = in.read()) == -1) return null;
>>             sbuf[35] = (byte)b;
>>
>>
>> which maybe can be replaced by
>>
>>                                         /* out of phase maybe? */
>>                           /* inch forward 1 byte and try again */
>>             if (log.level>= 5) {
>>                 log.println( "maybe we are out of phase. Try resyncing" );
>>             }
>>             System.arraycopy( sbuf, 1, sbuf, 0, 35 );
>>             if (log.level>= 6) {
>>                 jcifs.util.Hexdump.hexdump( log, sbuf, 4, 31 );
>>             }
>>             if (readn( in, sbuf, 35, 1 ) == -1) {
>>                 return null;
>>             }
>>
>> Is "out of phase" an often case?
> Hi Sebastian,
>
> No. It should never happen. I'm not sure if that code has ever even
> been tested. I think the purpose of that code is to gracefully handle
> skipping a message that JCIFS could not decode for some reason. So
> there is really no reason for "improvement" here.
>
> Mike
>
I think i understand why there is a resync-phase. In doSkip() which is 
called
when something went wrong (if there is no response object matching the
peeked key) jcifs skips some bytes from Inputstream.

Here is the code:
     protected void doSkip() throws IOException {
         int size = Encdec.dec_uint16be( sbuf, 2 ) & 0xFFFF;
         if (size < 33 || (4 + size) > rcv_buf_size ) {
             /* log message? */
             in.skip( in.available() );
         } else {
             in.skip( size - 32 );
         }
     }

The else part is pretty clear. Just skip the bytes of this message and 
go on. skip() javadoc says
that you should not rely on the amount of bytes skipped but at least on 
socketstreams it is somewhat
relayable that is skippes the number of bytes specified.

But
in.skip( in.avaiable());
seems to be little bit weak out of my point of view.

in.avaiable will return some (more or less predictable size of bytes 
avaiable on a first skip/read call)
but it is not the number of bytes send from the server to the client. It 
is the amount of bytes that in filled into the OM(VM)-Level Inputbuffer 
for this socketconnection.  And this buffer can be smaller than
size of the message. So i think in case of "(4 + size) > rcv_buf_size" 
jcifs should skip also "size-32" bytes and not just the avaiable bytes.
In case of "size < 33" (which is the case when something went really 
wrong because size cannot by smaller than 33, isn't it?) jcifs should 
skip only 1 byte and try to resync to not miss to many messages.
There is a theoretical risk to resync to early . But just skipping all 
bytes in inputbuffer has the same risk of an shifted resync.

Just in the case jcifs need a faster resync. Here is my proposal to make 
it happen.

replace this in SMBTransport.peekKey()
                                         /* out of phase maybe? */
                           /* inch forward 1 byte and try again */
             if (log.level >= 5) {
                 log.println( "maybe we are out of phase. Try resyncing" );
             }
             System.arraycopy( sbuf, 1, sbuf, 0, 35 );
             if (log.level >= 6) {
                 jcifs.util.Hexdump.hexdump( log, sbuf, 4, 31 );
             }
             if (readn( in, sbuf, 35, 1 ) == -1) {
                 return null;
             }


with this
                                         /* out of phase maybe? */
                           /* inch forward 1 byte and try again */
             if (log.level >= 5) {
                 log.println( "maybe we are out of phase. Try resyncing" );
             }
             int offset = searchResyncPoint();
             if (offset <36) {
                 // If there are some bytes left after resync point
                 // copy to front
                 System.arraycopy( sbuf, offset, sbuf, 0, 36-offset );
             }
             // Refill buffer to 36 bytes. If there are not enough bytes 
left
             // there is no key that could be returned
             if (readn( in, sbuf, 36-offset, offset ) < offset) {
                 return null;
             }
             if (log.level >= 6) {
                 jcifs.util.Hexdump.hexdump( log, sbuf, 4, 32 );
             }

and add this method to SmbTransport

     private int searchResyncPoint() {
         final byte[] pattern = new byte[]{(byte)0x00
                 ,(byte)0x00
                 ,(byte)0x00 // Not checked
                 ,(byte)0x00 // Not checked
                 ,(byte)0xFF
                 ,(byte)'S'
                 ,(byte)'M'
                 ,(byte)'B'
                 };
         for(int i=1; i<36; i++) {
             int amountOfMatch = 0;
             while (i+amountOfMatch < 36
&& amountOfMatch < pattern.length
&& sbuf[i+amountOfMatch] == pattern[amountOfMatch]) {
                 amountOfMatch += (amountOfMatch == 1) ? 3 : 1;
             }
             if (i+amountOfMatch == 36 || amountOfMatch == pattern.length) {
                 return i;
             }
         }
         return 36;
     }



-- Sebastian