tonyxiahua / sandrop

Automatically exported from code.google.com/p/sandrop
0 stars 0 forks source link

possible bugs in Message.flushContentStream() #108

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. in package org.sandrop.webscarab.model, 
the method: Message.flushContentStream(OutputStream os)

        int got = _contentStream.read(buf);
        int sum = got; 
        _logger.finest("Got " + got + " bytes");
        while (got > 0) {
                .....

I think here should be "got >= 0"
In java InputStream, it "Returns the number of bytes actually read or -1 if the 
end of the stream has been reached."

Since it is a network process, delay unavailable.

2. Better have a version of this method with Content-Length used.

Suppose that, on internet, these are some web servers or gateways, when have 
all data transfered, but still waiting for client to close the connection.(for 
reason of TCP closing status transfering, heavy loaded servers are more hoping 
for client to close the connection first, I think it is also why the HTTP 
"Content-Length" used)
So if Content-Length is got from server, flushContentStream() can use it to 
compare with variable "sum" in the function, to find the first time to return.

What is the expected output? What do you see instead?

What version of the product are you using? On what operating system?

Please provide any additional information below.

Original issue reported on code.google.com by harr...@gmail.com on 21 May 2014 at 8:42

GoogleCodeExporter commented 8 years ago
Another bug of here:
        int got = _contentStream.read(buf);
        int sum = got; 
        _logger.finest("Got " + got + " bytes");
        while (got >= 0) {
            sum += got;

here, "int sum = got; " got the wrong place, the first read size will be added 
twice.

if variable "sum" is used in future, it should be resolved.

Original comment by harr...@gmail.com on 21 May 2014 at 8:49

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
/************here is my implementation of limit length of  response content 
read ***************/
    public byte[] getContent() {
        try {
            flushContentStream(null);
            if (getContentSize() > MessageOutputStream.LARGE_CONTENT_SIZE){
                return CONTENT_TOO_BIG;
            }
        } catch (IOException ioe) {
            _logger.info("IOException flushing the contentStream: " + ioe);
        }

        return decodeContent();
    }

    /**
     * Harry add it, with length limited content read
     */
    public byte[] getResponseContent() {
        try {
            int length = getHeaderInt("Content-Length", -1);
            flushContentStream(null, length);
            if (getContentSize() > MessageOutputStream.LARGE_CONTENT_SIZE){
                return CONTENT_TOO_BIG;
            }
        } catch (IOException ioe) {
            _logger.info("IOException flushing the contentStream: " + ioe);
        }

        return decodeContent();
    }

    public byte[] decodeContent() {
        if (_content != null && _gzipped) {
            try {
                InputStream is = getContentInputStream();
        ....
        ....
    }

    public int getHeaderInt(String name, int default_value) {
        int ret = default_value;
        String value = getHeader(name);
        if (value != null) {
            try {
                ret = Integer.parseInt(value);
            } catch (NumberFormatException e) {
                _logger.finest("Bad HTTP Header - " + name + ": " + value);
            }
        }

        return ret;
    }

    /**
     *  WITH LENGTH LIMITED CONTENT READ
     * @param os
     * @param length        limit length will read from response. It should be >= 0, if length < 0, then no limit length defined
     * @throws IOException
     */
    private void flushContentStream(OutputStream os, int length) throws IOException {
        IOException ioe = null;
        if (_contentStream == null) return;
        _content = new MessageOutputStream();
        byte[] buf = new byte[4096];
        _logger.finest("Reading initial bytes from contentStream " + _contentStream);

        int got = _contentStream.read(buf, 0, (length >= 0) ? Math.min(length, buf.length) : buf.length);
        int sum = got; 
        _logger.finest("Got " + got + " bytes");
        while (got >= 0) {
            _content.write(buf,0, got);
            if (os != null) {
                try {
                    os.write(buf,0,got);
                } catch (IOException e) {
                    _logger.info("IOException ioe writing to output stream : " + e);
                    // _logger.info("Had seen " + (_content.size()-got) + " bytes, was writing " + got);
                    ioe = e;
                    os = null;
                }
            }else{
                if (LOGD) Log.d(TAG, "output Stream is null");
            }

            // if length is available(>=0) and read length enough, just return 
            if (length >= 0 && sum >= length)
                break;

            got = _contentStream.read(buf, 0, (length >= 0) ? Math.min(length, buf.length) : buf.length);
            sum += got;
            _logger.finest("Got " + got + " bytes");
        }
        _content.flush();
        _contentStream = null;
        if (ioe != null) throw ioe;
    }

Original comment by harr...@gmail.com on 21 May 2014 at 10:00

GoogleCodeExporter commented 8 years ago
phu... you are finding a lot of bugs in this flushContentStream() :)
I'm really appreciate.
will look at it and fix it ASAP

Original comment by supp.san...@gmail.com on 21 May 2014 at 3:17

GoogleCodeExporter commented 8 years ago
These bugs are also manifested in real use of code?

Original comment by supp.san...@gmail.com on 21 May 2014 at 3:19

GoogleCodeExporter commented 8 years ago

Original comment by supp.san...@gmail.com on 21 May 2014 at 3:19

GoogleCodeExporter commented 8 years ago
the first one is better to fix, whether read() will return "0" relay to the 
implementation of java.util, perhaps in some device the problem will be 
triggered?
(as i know, in Async socket connection, read can return 0)

the second one is something to accelerate the return of read().

Original comment by harr...@gmail.com on 22 May 2014 at 10:29

GoogleCodeExporter commented 8 years ago
"            sum += got;"

 perhaps it should be : 

"     if (got > 0) sum += got;"

Original comment by harr...@gmail.com on 22 May 2014 at 10:33

GoogleCodeExporter commented 8 years ago
Yes, all this came up if sum is used for deciding when to end.
Don't see any harm also to expect that read returns 0 in some cases.
Will try to incorporate your fixes in code.

Original comment by supp.san...@gmail.com on 22 May 2014 at 4:20