[jcifs] jcifs bug

Bernie Wieser bernie.wieser at calgary.qcdata.com
Wed Mar 27 08:05:15 EST 2002


I've found why the SmbFileInputStream hangs...  and it is 100% reproducable
for me.  So I've
fixed it.

The problem:
SmbFileInputStream goes into an endless loop for short files.

The cause:
A short read never causes EOF to be reached.


So, I've modified the code as follows:
1) if I read with zero return, assuming valid non blocking not ready
2) if I get a short read, assume no partial reads and set eof
3) don't let myself get called twice if eof set

B.


/* jcifs smb client library in Java
 * Copyright (C) 2000  "Michael B. Allen" <mballen at erols.com>
 *
 * This library is free software; you can redistribute it and/or
 * modify it under the terms of the GNU Lesser General Public
 * License as published by the Free Software Foundation; either
 * version 2.1 of the License, or (at your option) any later version.
 *
 * This library is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * Lesser General Public License for more details.
 *
 * You should have received a copy of the GNU Lesser General Public
 * License along with this library; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 */

package jcifs.smb;

import java.io.InputStream;
import java.io.IOException;
import java.net.UnknownHostException;
import java.net.MalformedURLException;

/**
 * This InputStream can read bytes from a file on an SMB file server.
 */

public class SmbFileInputStream extends InputStream {

    private SmbFile file;
    private int fp, off, readSize, openFlags;
    private byte[] tmp = new byte[1];
    private boolean eof;

/**
 * Creates an {@link java.io.InputStream} for reading bytes from a file on
 * an SMB server addressed by the <code>url</code> parameter. See {@link
 * jcifs.smb.SmbFile} for a detailed description and examples of the smb
 * URL syntax.
 *
 * @param url An smb URL string representing the file to read from
 * @return A new <code>InputStream</code> for the specified
<code>SmbFile</code>
 */

    public SmbFileInputStream( String url ) throws SmbException,
MalformedURLException, UnknownHostException {
        this( new SmbFile( url ));
    }

/**
 * Creates an {@link java.io.InputStream} for reading bytes from a file on
 * an SMB server represented by the {@link jcifs.smb.SmbFile} parameter. See
 * {@link jcifs.smb.SmbFile} for a detailed description and examples of
 * the smb URL syntax.
 *
 * @param url An smb URL string representing the file to write to
 * @return A new <code>InputStream</code> for the specified
<code>SmbFile</code>
 */

    public SmbFileInputStream( SmbFile file ) throws SmbException,
MalformedURLException, UnknownHostException {
        this( file, SmbFile.O_RDONLY );
    }

    SmbFileInputStream( SmbFile file, int openFlags ) throws SmbException,
MalformedURLException, UnknownHostException {
        this.file = file;
        this.openFlags = openFlags;
        file.open( openFlags );
        readSize = Math.min( file.tree.session.transport.rcv_buf_size - 70,


                          file.tree.session.transport.server.maxBufferSize -
70 );
        this.fp = 0;
        this.eof = false;
    }

/**
 * Closes this input stream and releases any system resources associated
with the stream.
 *
 * @throws IOException if a network error occurs
 */

    public void close() throws IOException {
        file.close();
    }

/**
 * Reads a byte of data from this input stream.
 *
 * @throws IOException if a network error occurs
 */

    public int read() throws IOException {
        // need oplocks to cache otherwise use BufferedInputStream
        if( read( tmp, 0, 1 ) == -1 ) {
            return -1;
        }
        return tmp[0] & 0xFF;
    }

/**
 * Reads up to b.length bytes of data from this input stream into an array
of bytes.
 *
 * @throws IOException if a network error occurs
 */

    public int read( byte[] b ) throws IOException {
        return read( b, 0, b.length );
    }

/**
 * Reads up to len bytes of data from this input stream into an array of
bytes.
 *
 * @throws IOException if a network error occurs
 */

    public int read( byte[] b, int off, int len ) throws IOException {
        if( eof )
          return -1;

        if( len <= 0 ) {
            return 0;
        }
        int start = fp;

        // ensure file is open
        file.open( openFlags );

        Log.println( Log.WARNINGS, "smb read warning",
                " fid=" + file.fid + ",off=" + off + ",len=" + len );

        /*
         * Read AndX Request / Response
         */

        SmbComReadAndXResponse response = new SmbComReadAndXResponse( b,
off );

        if( file.isPipe ) {
            response.responseTimeout = 0;
        }

        int r, n;
        do {
            r = len > readSize ? readSize : len;
            try {
                file.send( new SmbComReadAndX( file.fid, fp, r, null ),
response );
            } catch( SmbException se ) {
            eof = true;
                if( file.isPipe && se.errorClass == SmbException.ERRDOS &&
                                            se.errorCode ==
SmbException.ERRbrokenpipe ) {

                    return -1;
                }
                throw se;
            }
            n = response.dataLength;
            if( n == 0 ) // non blocking...
              continue;
            if( r != n ) // short read, assume no partial reads
              eof = true;
            if( n < 0 ) // some error (eof never reported)
              break;


            fp += n;
            len -= n;
            response.off += n;
        } while( len > 0 && !eof );
int rv = fp - start;
        return rv;
    }
}



-----Original Message-----
From: Michael B. Allen [mailto:mballen at erols.com]
Sent: Friday, March 22, 2002 4:46 PM
To: Bernie Wieser; crh at ubiqx.mn.org
Cc: jcifs at samba.org
Subject: Re: [jcifs] jcifs bug


On Fri, 22 Mar 2002 16:16:34 -0700
"Bernie Wieser" <bernie.wieser at calgary.qcdata.com> wrote:

> Yes, but even that is dangerous!  Take the following....
>
> URL u = new URL( "smb://host/...")
>
> it will call parse right?  what if host isn't known?  then
> parse consumes the exception, does not set u, so u is bogus.

Not bogus but malformed. If you run a simple test you get:

Exception in thread "main" java.net.MalformedURLException: smb:, null
        at jcifs.smb.SmbURL.parseSmbURL(SmbURL.java:223)
        at jcifs.smb.SmbFile.<init>(SmbFile.java:385)
        at jcifs.smb.SmbFile.<init>(SmbFile.java:338)
        at jcifs.smb.Handler.openConnection(Handler.java:29)
        at java.net.URL.openConnection(URL.java:781)
        at java.net.URL.openStream(URL.java:798)
        at GetURL.main(GetURL.java:30)

I think that should be ok but I don't know how it will perform when
stressed. Again, the SMB Protocol handler has not been thoroughly
tested/explored (that's why I'm so glad to see you using it the way
you are).

> On the one hand, the cached info would yield a corrupt url,
> and in the new version, an unknown host or other exception
> will yield an uninitialized url, so...
>
> Someone (Me?) needs to write parse url so it doesn't actually
> try to do anything (i.e. nmblookup).  All it must do is scan...
>
> To solve our current problem, I end up rethrowing the smb exception
> as a runtime exception in parse (evil hack until fixed).

There's a noteworthy problem here. The jCIFS SMB URL is based on an IETF
draft that requires the authority component to be resolved *during the
parse* in order to determine if it's a servername like:

  smb://server

or a workgroup name:

  smb://mygroup

You can see this URL is "overloaded". So a query (actually two concurrent
queries) must go out on the wire. Otherwise there would be no reason to
go out on the wire; name resolution could be handled lazily.

Chris,

Are you even considering the:

  smb:mygroup

format suggested by the other URL IETF people?

Mike

>
> B.
>
>
>
> -----Original Message-----
> From: Michael B. Allen [mailto:mballen at erols.com]
> Sent: Friday, March 22, 2002 4:16 PM
> To: Bernie Wieser
> Subject: Re: [jcifs] jcifs bug
>
>
> You had it in the try/catch in your code. I'm following you man :~)
>
>
> On Fri, 22 Mar 2002 16:05:09 -0700
> "Bernie Wieser" <bernie.wieser at calgary.qcdata.com> wrote:
>
> >
> > Another bug that you've fixed...
> >
> > By moving setUrl inside the try/catch block, process will no longer
> > call setUrl with cached file information (so ie UnknownHost will not set
> > the URL to the wrong thing.)   We just bumped into this one.
> >
> > B.

--
May The Source be with you.






More information about the jcifs mailing list