Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncated responses #6

Open
cergfix opened this issue Jun 29, 2015 · 2 comments
Open

Truncated responses #6

cergfix opened this issue Jun 29, 2015 · 2 comments

Comments

@cergfix
Copy link

cergfix commented Jun 29, 2015

There appears to be a problem with

//read response data
ByteBuffer buffer = ByteBuffer.allocate(10240);
connection.read(buffer);
buffer.flip();

Sometimes, when the response is quite big, it's being truncated even if ByteBuffer.allocate is set to a higher value.

Any ideas? I will try to dig as much debug data as possible for this.

@serphen
Copy link

serphen commented Jun 29, 2015

Hi,

The problem is because of multiplexed STDOUT and STDERR. When a STDERR answer comes in the middle it breaks everything.

Steps to reproduce:

Create a PHP file with:

<a href="">....</a>
</li>
<li>
<a href="">....</a>
</li>
<li>BREAKME
<?php
fprintf(stderr, "TROLOLOLO");
?>
<a href="">....</a>
</li>END

The answer will stop at BREAKME instead of stopping at END

The problem comes from the line: https://github.com/Happyr/fcgi4j/blob/master/src/com/googlecode/fcgi4j/FCGIConnection.java#L484
which is early breaking (it breaks at the first non-STDOUT packet).

Proposed solution (I have refactored a bit read() as well to be consistent with read with ByteBuffers):

    @Override
    public int read(ByteBuffer dst) throws IOException {
        readyRead();

        int read = 0;

        int available = 0, padding = 0;

        outer:
        if (true) {
            if (available == 0) {
                read += readBufferedData(dst);
            } else {
                read += readStdoutData(dst, available, padding);
            }

            while (dst.hasRemaining()) {
                FCGIHeader header = readHeader();

    ----->            if ((header.getType() == FCGIHeaderType.FCGI_STDOUT || header.getType() == FCGIHeaderType.FCGI_STDERR) && header.getLength() != 0) {
                    int currentRead = readStdoutData(dst, header.getLength(), header.getPadding());

                    available = header.getLength() - currentRead;
                    padding = header.getPadding();

                    read += currentRead;
                } else {
                    if (header.getType() == FCGIHeaderType.FCGI_END_REQUEST) {
                        finishRequest(header);
                        break outer;
                    }
                }
            }
        }

        if (available != 0) {
            bufferStdoutData(available, padding);
        }

        return read;
    }

@Nyholm
Copy link
Member

Nyholm commented Jun 30, 2015

Fantastic! Thank you for reporting this bug and thank you @serphen for providing a fix. Would you mind sending a pull request with the changes?

cergfix pushed a commit to cergfix/fcgi4j that referenced this issue Jul 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants