github-code-scanning[bot] commented on code in PR #13609:
URL: https://github.com/apache/druid/pull/13609#discussion_r1055009024
##########
server/src/main/java/org/apache/druid/server/QueryResultPusher.java:
##########
@@ -92,23 +101,45 @@
public abstract void writeException(Exception e, OutputStream out) throws
IOException;
- public void push()
+ /**
+ * Pushes results out. Can sometimes return a JAXRS Response object instead
of actually pushing to the output
+ * stream, primarily for error handling that occurs before switching the
servlet to asynchronous mode.
+ *
+ * @return null if the response has already been handled and pushed out, or
a non-null Response object if it expects
+ * the container to put the bytes on the wire.
+ */
+ @Nullable
+ public Response push()
{
- response.setHeader(QueryResource.QUERY_ID_RESPONSE_HEADER, queryId);
-
ResultsWriter resultsWriter = null;
try {
resultsWriter = start();
-
- final QueryResponse<Object> queryResponse =
resultsWriter.start(response);
- if (queryResponse == null) {
- // It's already been handled...
- return;
+ final Response.ResponseBuilder startResponse = resultsWriter.start();
+ if (startResponse != null) {
+ startResponse.header(QueryResource.QUERY_ID_RESPONSE_HEADER, queryId);
+ for (Map.Entry<String, String> entry : extraHeaders.entrySet()) {
+ startResponse.header(entry.getKey(), entry.getValue());
+ }
+ return startResponse.build();
}
+ final QueryResponse<Object> queryResponse =
resultsWriter.getQueryResponse();
final Sequence<Object> results = queryResponse.getResults();
+ // We use an async context not because we are actually going to run this
async, but because we want to delay
+ // the decision of what the response code should be until we have gotten
the first few data points to return.
+ // Returning a Response object from this point forward requires that
object to know the status code, which we
+ // don't actually know until we are in the accumulator, but if we try to
return a Response object from the
+ // accumulator, we cannot properly stream results back, because the
accumulator won't release control of the
+ // Response until it has consumed the underlying Sequence.
+ asyncContext = request.startAsync();
+ response = (HttpServletResponse) asyncContext.getResponse();
+ response.setHeader(QueryResource.QUERY_ID_RESPONSE_HEADER, queryId);
Review Comment:
## HTTP response splitting
This header depends on a [user-provided value](1), which may cause a
response-splitting vulnerability.
[Show more
details](https://github.com/apache/druid/security/code-scanning/3571)
##########
server/src/main/java/org/apache/druid/server/QueryResultPusher.java:
##########
@@ -92,23 +101,45 @@
public abstract void writeException(Exception e, OutputStream out) throws
IOException;
- public void push()
+ /**
+ * Pushes results out. Can sometimes return a JAXRS Response object instead
of actually pushing to the output
+ * stream, primarily for error handling that occurs before switching the
servlet to asynchronous mode.
+ *
+ * @return null if the response has already been handled and pushed out, or
a non-null Response object if it expects
+ * the container to put the bytes on the wire.
+ */
+ @Nullable
+ public Response push()
{
- response.setHeader(QueryResource.QUERY_ID_RESPONSE_HEADER, queryId);
-
ResultsWriter resultsWriter = null;
try {
resultsWriter = start();
-
- final QueryResponse<Object> queryResponse =
resultsWriter.start(response);
- if (queryResponse == null) {
- // It's already been handled...
- return;
+ final Response.ResponseBuilder startResponse = resultsWriter.start();
+ if (startResponse != null) {
+ startResponse.header(QueryResource.QUERY_ID_RESPONSE_HEADER, queryId);
+ for (Map.Entry<String, String> entry : extraHeaders.entrySet()) {
+ startResponse.header(entry.getKey(), entry.getValue());
+ }
+ return startResponse.build();
}
+ final QueryResponse<Object> queryResponse =
resultsWriter.getQueryResponse();
final Sequence<Object> results = queryResponse.getResults();
+ // We use an async context not because we are actually going to run this
async, but because we want to delay
+ // the decision of what the response code should be until we have gotten
the first few data points to return.
+ // Returning a Response object from this point forward requires that
object to know the status code, which we
+ // don't actually know until we are in the accumulator, but if we try to
return a Response object from the
+ // accumulator, we cannot properly stream results back, because the
accumulator won't release control of the
+ // Response until it has consumed the underlying Sequence.
+ asyncContext = request.startAsync();
+ response = (HttpServletResponse) asyncContext.getResponse();
+ response.setHeader(QueryResource.QUERY_ID_RESPONSE_HEADER, queryId);
+ for (Map.Entry<String, String> entry : extraHeaders.entrySet()) {
+ response.setHeader(entry.getKey(), entry.getValue());
Review Comment:
## HTTP response splitting
This header depends on a [user-provided value](1), which may cause a
response-splitting vulnerability.
[Show more
details](https://github.com/apache/druid/security/code-scanning/3572)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
