Skip to content

Commit

Permalink
fix(http): fix rejects of HTTP GET requests when content-length set t…
Browse files Browse the repository at this point in the history 
…o 0 (#4242)
  • Loading branch information
@ideoma
ideoma committed Feb 28, 2024
1 parent b8fbf92 commit 639865c
Show file tree
Hide file tree
Showing 8 changed files with 153 additions and 52 deletions.
2 changes: 2 additions & 0 deletions compat/src/test/java/io/questdb/compat/InfluxDBClientTest.java
View file
Expand Up @@ -477,8 +477,10 @@ public void testNoErrorLastLineNoLineBreak() throws Exception {

@Test
public void testPing() {
LOG.info().$("=== send fragmentation=").$(5).$();
try (final ServerMain serverMain = ServerMain.create(root, new HashMap<String, String>() {{
put(PropertyKey.LINE_HTTP_PING_VERSION.getEnvVarName(), "v2.2.2");
put(PropertyKey.DEBUG_FORCE_SEND_FRAGMENTATION_CHUNK_SIZE.getEnvVarName(), "5");
}})) {
serverMain.start();
try (final InfluxDB influxDB = InfluxDBUtils.getConnection(serverMain)) {
Expand Down
70 changes: 53 additions & 17 deletions core/src/main/java/io/questdb/cutlass/http/HttpConnectionContext.java
View file
Expand Up @@ -777,6 +777,42 @@ private HttpRequestProcessor getHttpRequestProcessor(HttpRequestProcessorSelecto
return processor;
}

private HttpRequestProcessor checkProcessorValidForRequest(
Utf8Sequence method,
HttpRequestProcessor processor,
boolean chunked,
boolean multipartRequest,
long contentLength,
boolean multipartProcessor
) {
if (Utf8s.equalsAscii("POST", method) || Utf8s.equalsAscii("PUT", method)) {
if (!multipartProcessor) {
if (multipartRequest) {
return rejectRequest(HTTP_NOT_FOUND, "Method (multipart POST) not supported");
} else {
return rejectRequest(HTTP_NOT_FOUND, "Method not supported");
}
}
if (chunked && contentLength > 0) {
return rejectRequest(HTTP_BAD_REQUEST, "Invalid chunked request; content-length specified");
}
if (!chunked && !multipartRequest && contentLength < 0) {
return rejectRequest(HTTP_BAD_REQUEST, "Content-length not specified for POST/PUT request");
}
} else if (Utf8s.equalsAscii("GET", method)) {
if (chunked || multipartRequest || contentLength > 0) {
return rejectRequest(HTTP_BAD_REQUEST, "GET request method cannot have content");
}
if (multipartProcessor) {
return rejectRequest(HTTP_NOT_FOUND, "Method GET not supported");
}
} else {
return rejectRequest(HTTP_BAD_REQUEST, "Method not supported");
}

return processor;
}

private boolean handleClientRecv(HttpRequestProcessorSelector selector, RescheduleContext rescheduleContext) throws PeerIsSlowToReadException, PeerIsSlowToWriteException, ServerDisconnectException {
boolean busyRecv = true;
try {
Expand Down Expand Up @@ -844,29 +880,22 @@ private boolean handleClientRecv(HttpRequestProcessorSelector selector, Reschedu
}
}

processor = checkProcessorValidForRequest(
headerParser.getMethod(),
processor,
chunked,
multipartRequest,
contentLength,
multipartProcessor
);

if (chunked) {
if (!multipartProcessor) {
// bad request - regular request for processor that expects multipart
processor = rejectRequest(HTTP_NOT_FOUND, "method (chunked POST) not supported");
}
busyRecv = consumeChunked(processor, headerEnd, read, newRequest);
} else if (multipartRequest) {
if (!multipartProcessor) {
// bad request - multipart request for processor that doesn't expect multipart
processor = rejectRequest(HTTP_NOT_FOUND, "method (multipart POST) not supported");
}
busyRecv = consumeMultipart(socket, processor, headerEnd, read, newRequest, rescheduleContext);
} else if (contentLength > -1) {
if (!multipartProcessor) {
processor = rejectRequest(HTTP_NOT_FOUND, "method (POST) not supported");
}
} else if (contentLength > 0) {
busyRecv = consumeContent(contentLength, socket, processor, headerEnd, read, newRequest);
} else {
if (multipartProcessor) {
// bad request - regular request for processor that expects multipart
processor = rejectRequest(HTTP_BAD_REQUEST, "Bad request. Multipart POST expected.");
}

// Do not expect any more bytes to be sent to us before
// we respond back to client. We will disconnect the client when
// they abuse protocol. In addition, we will not call processor
Expand Down Expand Up @@ -1019,5 +1048,12 @@ public void onRequestComplete(HttpConnectionContext context) throws PeerDisconne
}
reset();
}

@Override
public void resumeSend(
HttpConnectionContext context
) throws PeerDisconnectedException, PeerIsSlowToReadException, ServerDisconnectException, QueryPausedException {
onRequestComplete(context);
}
}
}
69 changes: 44 additions & 25 deletions core/src/main/java/io/questdb/cutlass/http/HttpResponseSink.java
View file
Expand Up @@ -91,6 +91,7 @@ public void clear() {
totalBytesSent = 0;
headersSent = false;
chunkedRequestDone = false;
simple.clear();
resetZip();
}

Expand Down Expand Up @@ -748,6 +749,14 @@ private void putAsciiInternal(@Nullable CharSequence cs) {
}

public class SimpleResponseImpl {
private boolean contentSent = false;
private boolean headerSent = false;

public void clear() {
contentSent = false;
headerSent = false;
}

public void sendStatusJsonContent(
int code
) throws PeerDisconnectedException, PeerIsSlowToReadException {
Expand All @@ -772,12 +781,15 @@ public void sendStatusJsonContent(
}

public void sendStatusNoContent(int code, @Nullable CharSequence header) throws PeerDisconnectedException, PeerIsSlowToReadException {
buffer.clearAndPrepareToWriteToBuffer();
headerImpl.status(httpVersion, code, null, -2L);
if (header != null) {
headerImpl.put(header).put(Misc.EOL);
if (!headerSent) {
buffer.clearAndPrepareToWriteToBuffer();
headerImpl.status(httpVersion, code, null, -2L);
if (header != null) {
headerImpl.put(header).put(Misc.EOL);
}
prepareHeaderSink();
headerSent = true;
}
prepareHeaderSink();
flushSingle();
}

Expand Down Expand Up @@ -836,28 +848,35 @@ private void sendStatusWithContent(
@Nullable CharSequence cookieValue,
long contentLength
) throws PeerDisconnectedException, PeerIsSlowToReadException {
buffer.clearAndPrepareToWriteToBuffer();
final String std = headerImpl.status(httpVersion, code, contentType, contentLength);
if (header != null) {
headerImpl.put(header).put(Misc.EOL);
if (!headerSent) {
buffer.clearAndPrepareToWriteToBuffer();
headerImpl.status(httpVersion, code, contentType, contentLength);
if (header != null) {
headerImpl.put(header).put(Misc.EOL);
}
if (cookieName != null) {
setCookie(cookieName, cookieValue);
}
prepareHeaderSink();
headerSent = true;
}
if (cookieName != null) {
setCookie(cookieName, cookieValue);

if (!contentSent) {
flushSingle();
buffer.clearAndPrepareToWriteToBuffer();
if (message == null) {
sink.put(httpStatusMap.get(code)).putEOL();
} else {
// this is ugly, add a putUtf16() method to the response sink?
final Utf8StringSink utf8Sink = tlSink.get();
utf8Sink.clear();
utf8Sink.put(message);
sink.put(utf8Sink).putEOL();
}
final boolean chunked = headerImpl.isChunked();
buffer.prepareToReadFromBuffer(chunked, chunked);
contentSent = true;
}
prepareHeaderSink();
flushSingle();
buffer.clearAndPrepareToWriteToBuffer();
if (message == null) {
sink.put(std).putEOL();
} else {
// this is ugly, add a putUtf16() method to the response sink?
final Utf8StringSink utf8Sink = tlSink.get();
utf8Sink.clear();
utf8Sink.put(message);
sink.put(utf8Sink).putEOL();
}
final boolean chunked = headerImpl.isChunked();
buffer.prepareToReadFromBuffer(chunked, chunked);
resumeSend();
}

Expand Down
6 changes: 6 additions & 0 deletions core/src/main/java/io/questdb/cutlass/http/client/HttpClient.java
View file
Expand Up @@ -252,6 +252,12 @@ public class Request implements Utf8Sink {
private int state;
private boolean urlEncode = false;

public Request DELETE() {
assert state == STATE_REQUEST;
state = STATE_URL;
return put("DELETE ");
}

public Request GET() {
assert state == STATE_REQUEST;
state = STATE_URL;
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/io/questdb/cutlass/http/processors/LineHttpPingProcessor.java
View file
Expand Up @@ -26,4 +26,9 @@ public void onRequestComplete(
) throws PeerDisconnectedException, PeerIsSlowToReadException, ServerDisconnectException, QueryPausedException {
context.simpleResponse().sendStatusNoContent(204, header);
}

@Override
public void resumeSend(HttpConnectionContext context) throws PeerIsSlowToReadException, PeerDisconnectedException {
context.simpleResponse().sendStatusNoContent(204, header);
}
}
4 changes: 3 additions & 1 deletion core/src/main/java/io/questdb/cutlass/http/processors/TextImportProcessor.java
View file
Expand Up @@ -223,7 +223,9 @@ public void onPartEnd() throws PeerDisconnectedException, PeerIsSlowToReadExcept

@Override
public void onRequestComplete(HttpConnectionContext context) {
transientState.clear();
if (transientState != null) {
transientState.clear();
}
}

@Override
Expand Down
9 changes: 4 additions & 5 deletions core/src/test/java/io/questdb/test/cutlass/http/IODispatcherTest.java
View file
Expand Up @@ -54,7 +54,6 @@
import io.questdb.test.AbstractTest;
import io.questdb.test.CreateTableTestUtils;
import io.questdb.test.cairo.DefaultTestCairoConfiguration;
import io.questdb.cairo.CursorPrinter;
import io.questdb.test.cairo.TableModel;
import io.questdb.test.cairo.TestRecord;
import io.questdb.test.cutlass.NetUtils;
Expand Down Expand Up @@ -1461,14 +1460,14 @@ public void testImportBadJson() throws Exception {
@Test
public void testImportBadRequestGet() throws Exception {
testImport(
"HTTP/1.1 400 Bad request\r\n" +
"HTTP/1.1 404 Not Found\r\n" +
"Server: questDB/1.0\r\n" +
"Date: Thu, 1 Jan 1970 00:00:00 GMT\r\n" +
"Transfer-Encoding: chunked\r\n" +
"Content-Type: text/plain; charset=utf-8\r\n" +
"\r\n" +
"27\r\n" +
"Bad request. Multipart POST expected.\r\n" +
"1a\r\n" +
"Method GET not supported\r\n" +
"\r\n" +
"00\r\n" +
"\r\n",
Expand Down Expand Up @@ -5676,7 +5675,7 @@ public void testPostRequestToGetProcessor() throws Exception {
"Content-Type: text/plain; charset=utf-8\r\n" +
"\r\n" +
"27\r\n" +
"method (multipart POST) not supported\r\n" +
"Method (multipart POST) not supported\r\n" +
"\r\n" +
"00\r\n" +
"\r\n",
Expand Down
40 changes: 36 additions & 4 deletions core/src/test/java/io/questdb/test/cutlass/http/line/LineHttpFailureTest.java
View file
Expand Up @@ -27,6 +27,7 @@
import io.questdb.Bootstrap;
import io.questdb.DefaultBootstrapConfiguration;
import io.questdb.DefaultHttpClientConfiguration;
import io.questdb.ServerMain;
import io.questdb.cairo.TableToken;
import io.questdb.cairo.pool.PoolListener;
import io.questdb.cutlass.http.client.HttpClient;
Expand All @@ -48,6 +49,7 @@
import org.junit.Before;
import org.junit.Test;

import java.util.HashMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;

Expand Down Expand Up @@ -357,9 +359,10 @@ public void testClientDisconnectsMidRequest() throws Exception {
@Test
public void testPutAndGetAreNotSupported() throws Exception {
TestUtils.assertMemoryLeak(() -> {
try (final TestServerMain serverMain = startWithEnvVariables(
DEBUG_FORCE_SEND_FRAGMENTATION_CHUNK_SIZE.getEnvVarName(), "5"
)) {
try (final ServerMain serverMain = ServerMain.create(root, new HashMap<String, String>() {{
put(DEBUG_FORCE_SEND_FRAGMENTATION_CHUNK_SIZE.getEnvVarName(), "5");
}})
) {
serverMain.start();
String line = "line,sym1=123 field1=123i 1234567890000000000\n";

Expand Down Expand Up @@ -389,7 +392,36 @@ public void testPutAndGetAreNotSupported() throws Exception {
.send()
) {
resp.await();
TestUtils.assertEquals("404", resp.getStatusCode());
TestUtils.assertEquals("400", resp.getStatusCode());
}
}

try (HttpClient httpClient = HttpClientFactory.newPlainTextInstance(new DefaultHttpClientConfiguration())) {
HttpClient.Request request = httpClient.newRequest("localhost", serverMain.getHttpServerPort());
try (
HttpClient.ResponseHeaders resp = request.DELETE()
.url("/write ")
.withContent()
.putAscii(line)
.putAscii(line)
.send()
) {
resp.await();
TestUtils.assertEquals("400", resp.getStatusCode());
}
}

try (HttpClient httpClient = HttpClientFactory.newPlainTextInstance(new DefaultHttpClientConfiguration())) {
HttpClient.Request request = httpClient.newRequest("localhost", serverMain.getHttpServerPort());
try (
HttpClient.ResponseHeaders resp = request.POST()
.url("/write ")
.putAscii(line)
.putAscii(line)
.send()
) {
resp.await();
TestUtils.assertEquals("400", resp.getStatusCode());
}
}
}
Expand Down

0 comments on commit 639865c

Please sign in to comment.