FCGI improvements. (#9733)

* FCGI improvements.

* Better handling for HTTPS parameter on server side.
* Better handling of unknown frame types.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2023-05-15 15:04:50 +02:00 committed by GitHub
parent ac2765c98d
commit b884469103
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 171 additions and 67 deletions

View File

@ -69,7 +69,7 @@ public class FCGI
case 8 -> DATA;
case 9 -> GET_VALUES;
case 10 -> GET_VALUES_RESULT;
default -> throw new IllegalArgumentException();
default -> throw new IllegalArgumentException("unknown frame type " + code);
};
}

View File

@ -412,11 +412,6 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne
destroy();
}
protected void close(Throwable failure)
{
HttpConnectionOverFCGI.this.close(failure);
}
@Override
public boolean isClosed()
{
@ -524,7 +519,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne
public void onFailure(int request, Throwable failure)
{
if (LOG.isDebugEnabled())
LOG.debug("onFailure r={},f={}", request, failure);
LOG.debug("onFailure request={}", request, failure);
HttpChannelOverFCGI channel = HttpConnectionOverFCGI.this.channel;
if (channel != null)
{

View File

@ -25,6 +25,7 @@ public class ClientParser extends Parser
public ClientParser(Listener listener)
{
super(listener);
ResponseContentParser stdOutParser = new ResponseContentParser(headerParser, listener);
contentParsers.put(FCGI.FrameType.STDOUT, stdOutParser);
StreamContentParser stdErrParser = new StreamContentParser(headerParser, FCGI.StreamType.STD_ERR, listener);
@ -35,7 +36,10 @@ public class ClientParser extends Parser
@Override
protected ContentParser findContentParser(FCGI.FrameType frameType)
{
return contentParsers.get(frameType);
ContentParser contentParser = contentParsers.get(frameType);
if (contentParser == null)
throw new IllegalArgumentException("unsupported frame type " + frameType);
return contentParser;
}
public interface Listener extends Parser.Listener
@ -47,7 +51,6 @@ public class ClientParser extends Parser
private record EndRequestListener(Listener listener, StreamContentParser... streamParsers) implements Listener
{
@Override
public void onBegin(int request, int code, String reason)
{

View File

@ -59,79 +59,88 @@ public abstract class Parser
private static final Logger LOG = LoggerFactory.getLogger(Parser.class);
protected final HeaderParser headerParser = new HeaderParser();
private final Listener listener;
private State state = State.HEADER;
private int padding;
protected Parser(Listener listener)
{
this.listener = listener;
}
/**
* @param buffer the bytes to parse
* @return true if the caller should stop parsing, false if the caller should continue parsing
*/
public boolean parse(ByteBuffer buffer)
{
while (true)
try
{
switch (state)
while (true)
{
case HEADER:
switch (state)
{
if (!headerParser.parse(buffer))
return false;
state = State.CONTENT;
break;
}
case CONTENT:
{
ContentParser contentParser = findContentParser(headerParser.getFrameType());
if (headerParser.getContentLength() == 0)
case HEADER ->
{
padding = headerParser.getPaddingLength();
state = State.PADDING;
if (contentParser.noContent())
return true;
if (!headerParser.parse(buffer))
return false;
state = State.CONTENT;
}
else
case CONTENT ->
{
ContentParser.Result result = contentParser.parse(buffer);
if (LOG.isDebugEnabled())
LOG.debug("Parsed request {} content {} result={}", headerParser.getRequest(), headerParser.getFrameType(), result);
if (result == ContentParser.Result.PENDING)
ContentParser contentParser = findContentParser(headerParser.getFrameType());
if (headerParser.getContentLength() == 0)
{
// Not enough data, signal to read/parse more.
padding = headerParser.getPaddingLength();
state = State.PADDING;
if (contentParser.noContent())
return true;
}
else
{
ContentParser.Result result = contentParser.parse(buffer);
if (LOG.isDebugEnabled())
LOG.debug("Parsed request {} content {} result={}", headerParser.getRequest(), headerParser.getFrameType(), result);
if (result == ContentParser.Result.PENDING)
{
// Not enough data, signal to read/parse more.
return false;
}
if (result == ContentParser.Result.ASYNC)
{
// The content will be processed asynchronously, signal to stop
// parsing; the async operation will eventually resume parsing.
return true;
}
padding = headerParser.getPaddingLength();
state = State.PADDING;
}
}
case PADDING ->
{
if (buffer.remaining() >= padding)
{
buffer.position(buffer.position() + padding);
reset();
}
else
{
padding -= buffer.remaining();
buffer.position(buffer.limit());
return false;
}
if (result == ContentParser.Result.ASYNC)
{
// The content will be processed asynchronously, signal to stop
// parsing; the async operation will eventually resume parsing.
return true;
}
padding = headerParser.getPaddingLength();
state = State.PADDING;
}
break;
}
case PADDING:
{
if (buffer.remaining() >= padding)
{
buffer.position(buffer.position() + padding);
reset();
break;
}
else
{
padding -= buffer.remaining();
buffer.position(buffer.limit());
return false;
}
}
default:
{
throw new IllegalStateException();
default -> throw new IllegalStateException();
}
}
}
catch (Throwable x)
{
buffer.position(buffer.limit());
listener.onFailure(headerParser.getRequest(), x);
return true;
}
}
protected abstract ContentParser findContentParser(FCGI.FrameType frameType);

View File

@ -23,6 +23,7 @@ public class ServerParser extends Parser
public ServerParser(Listener listener)
{
super(listener);
contentParsers.put(FCGI.FrameType.BEGIN_REQUEST, new BeginRequestContentParser(headerParser, listener));
contentParsers.put(FCGI.FrameType.PARAMS, new ParamsContentParser(headerParser, listener));
contentParsers.put(FCGI.FrameType.STDIN, new StreamContentParser(headerParser, FCGI.StreamType.STD_IN, listener));
@ -31,7 +32,10 @@ public class ServerParser extends Parser
@Override
protected ContentParser findContentParser(FCGI.FrameType frameType)
{
return contentParsers.get(frameType);
ContentParser contentParser = contentParsers.get(frameType);
if (contentParser == null)
throw new IllegalArgumentException("unsupported frame type " + frameType);
return contentParser;
}
public interface Listener extends Parser.Listener

View File

@ -15,6 +15,8 @@ package org.eclipse.jetty.fcgi.parser;
import java.nio.ByteBuffer;
import java.util.Locale;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
@ -25,6 +27,8 @@ import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.io.ArrayByteBufferPool;
import org.eclipse.jetty.io.ByteBufferPool;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
@ -102,7 +106,7 @@ public class ClientParserTest
}
@Test
public void testParseNoResponseContent() throws Exception
public void testParseNoResponseContent()
{
int id = 13;
HttpFields fields = HttpFields.build()
@ -145,7 +149,7 @@ public class ClientParserTest
}
@Test
public void testParseSmallResponseContent() throws Exception
public void testParseSmallResponseContent()
{
int id = 13;
HttpFields.Mutable fields = HttpFields.build();
@ -196,7 +200,7 @@ public class ClientParserTest
}
@Test
public void testParseLargeResponseContent() throws Exception
public void testParseLargeResponseContent()
{
int id = 13;
HttpFields.Mutable fields = HttpFields.build();
@ -246,4 +250,66 @@ public class ClientParserTest
accumulator.release();
}
@ParameterizedTest
// Frame type 0x01 is BEGIN_REQUEST, cannot be received by clients.
// Frame type 0x7F is unknown to FCGI.
@ValueSource(ints = {0x01, 0x7F})
public void testClientUnknownFrameType(int frameType) throws Exception
{
CountDownLatch failureLatch = new CountDownLatch(1);
ClientParser parser = new ClientParser(new ClientParser.Listener()
{
@Override
public void onFailure(int request, Throwable failure)
{
failureLatch.countDown();
}
});
// See Parser for the FCGI record structure.
ByteBuffer byteBuffer = ByteBuffer.allocate(8)
.put((byte)1)
.put((byte)frameType)
.putShort((short)13)
.putShort((short)0)
.put((byte)0)
.put((byte)0)
.flip();
parser.parse(byteBuffer);
assertFalse(byteBuffer.hasRemaining());
assertTrue(failureLatch.await(5, TimeUnit.SECONDS));
}
@ParameterizedTest
// Frame type 0x06 is STDOUT, cannot be received by servers.
// Frame type 0x7F is unknown to FCGI.
@ValueSource(ints = {0x06, 0x7F})
public void testServerUnknownFrameType(int frameType) throws Exception
{
CountDownLatch failureLatch = new CountDownLatch(1);
ServerParser parser = new ServerParser(new ServerParser.Listener()
{
@Override
public void onFailure(int request, Throwable failure)
{
failureLatch.countDown();
}
});
// See Parser for the FCGI record structure.
ByteBuffer byteBuffer = ByteBuffer.allocate(8)
.put((byte)1)
.put((byte)frameType)
.putShort((short)13)
.putShort((short)0)
.put((byte)0)
.put((byte)0)
.flip();
parser.parse(byteBuffer);
assertFalse(byteBuffer.hasRemaining());
assertTrue(failureLatch.await(5, TimeUnit.SECONDS));
}
}

View File

@ -25,6 +25,7 @@ import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.fcgi.FCGI;
import org.eclipse.jetty.fcgi.server.ServerFCGIConnectionFactory;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.server.Connector;
@ -257,4 +258,27 @@ public class FastCGIProxyHandlerTest
assertEquals(HttpStatus.OK_200, response.getStatus());
assertArrayEquals(content, response.getContent());
}
@Test
public void testFCGISecure() throws Exception
{
start(true, new Handler.Abstract()
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
assertTrue(request.isSecure());
assertTrue(HttpScheme.HTTPS.is(request.getHttpURI().getScheme()));
callback.succeeded();
return true;
}
});
fcgiHandler.setFastCGISecure(true);
ContentResponse response = client.newRequest("localhost", proxyConnector.getLocalPort())
.path(proxyContext.getContextPath() + "/index.php")
.send();
assertEquals(HttpStatus.OK_200, response.getStatus());
}
}

View File

@ -35,6 +35,7 @@ import org.eclipse.jetty.server.HttpStream;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.NanoTime;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.thread.Invocable;
import org.slf4j.Logger;
@ -57,6 +58,7 @@ public class HttpStreamOverFCGI implements HttpStream
private String _path;
private String _query;
private String _version;
private String _secure;
private Content.Chunk _chunk;
private boolean _committed;
private boolean _shutdown;
@ -101,6 +103,8 @@ public class HttpStreamOverFCGI implements HttpStream
_query = value;
else if (FCGI.Headers.SERVER_PROTOCOL.equalsIgnoreCase(name))
_version = value;
else if (FCGI.Headers.HTTPS.equalsIgnoreCase(name))
_secure = value;
else
processField(field);
}
@ -108,8 +112,8 @@ public class HttpStreamOverFCGI implements HttpStream
public void onHeaders()
{
String pathQuery = URIUtil.addPathQuery(_path, _query);
// TODO https?
MetaData.Request request = new MetaData.Request(_method, HttpScheme.HTTP.asString(), hostPort, pathQuery, HttpVersion.fromString(_version), _headers, -1);
HttpScheme scheme = StringUtil.isEmpty(_secure) ? HttpScheme.HTTP : HttpScheme.HTTPS;
MetaData.Request request = new MetaData.Request(_method, scheme.asString(), hostPort, pathQuery, HttpVersion.fromString(_version), _headers, -1);
Runnable task = _httpChannel.onRequest(request);
_allHeaders.forEach(field -> _httpChannel.getRequest().setAttribute(field.getName(), field.getValue()));
// TODO: here we just execute the task.
@ -260,7 +264,6 @@ public class HttpStreamOverFCGI implements HttpStream
boolean shutdown = _shutdown = info.getHttpFields().contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString());
ByteBufferPool bufferPool = _generator.getByteBufferPool();
ByteBufferPool.Accumulator accumulator = new ByteBufferPool.Accumulator();
Flusher flusher = _connection.getFlusher();
if (head)