From 45f7f54454f1ab143c1df6bb34dd609015cc118b Mon Sep 17 00:00:00 2001 From: John Gozde Date: Fri, 21 Mar 2025 14:14:46 -0600 Subject: [PATCH 01/10] Support Content-Distribution in statement result API --- docs/api-reference/sql-api.md | 3 + .../sql/resources/SqlStatementResource.java | 30 +++++++-- .../SqlMSQStatementResourcePostTest.java | 15 +++++ .../resources/SqlStatementResourceTest.java | 64 +++++++++++++++---- 4 files changed, 96 insertions(+), 16 deletions(-) diff --git a/docs/api-reference/sql-api.md b/docs/api-reference/sql-api.md index 2b25bdd02f0b..cead5f6040d1 100644 --- a/docs/api-reference/sql-api.md +++ b/docs/api-reference/sql-api.md @@ -1286,6 +1286,9 @@ Getting the query results for an ingestion query returns an empty response. * `resultFormat` (optional) * Type: String * Defines the format in which the results are presented. The following options are supported `arrayLines`,`objectLines`,`array`,`object`, and `csv`. The default is `object`. +* `filename` (optional) + * Type: String + * If set, attaches a `Content-Disposition` header to the response with the value of `attachment; filename={filename}`. #### Responses diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java index dcaa447539b1..1d7a3c3f2b3c 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java @@ -122,6 +122,7 @@ public class SqlStatementResource { public static final String RESULT_FORMAT = "__resultFormat"; + public static final String CONTENT_DISPOSITION_RESPONSE_HEADER = "Content-Disposition"; private static final Logger log = new Logger(SqlStatementResource.class); private final SqlStatementFactory msqSqlStatementFactory; private final ObjectMapper jsonMapper; @@ -277,6 +278,7 @@ public Response doGetResults( @PathParam("id") final String queryId, @QueryParam("page") Long page, @QueryParam("resultFormat") String resultFormat, + @QueryParam("filename") String filename, @Context final HttpServletRequest req ) { @@ -309,10 +311,18 @@ public Response doGetResults( ); throwIfQueryIsNotSuccessful(queryId, statusPlus); + final String contentDispositionHeaderValue = filename != null ? String.format("attachment; filename=%s", filename) : null; + Optional> signature = SqlStatementResourceHelper.getSignature(msqControllerTask); if (!signature.isPresent() || MSQControllerTask.isIngestion(msqControllerTask.getQuerySpec())) { // Since it's not a select query, nothing to return. - return Response.ok().build(); + final Response.ResponseBuilder responseBuilder = Response.ok(); + + if (contentDispositionHeaderValue != null) { + responseBuilder.header(CONTENT_DISPOSITION_RESPONSE_HEADER, contentDispositionHeaderValue); + } + + return responseBuilder.build(); } // returning results @@ -321,18 +331,30 @@ public Response doGetResults( results = getResultYielder(queryId, page, msqControllerTask, closer); if (!results.isPresent()) { // no results, return empty - return Response.ok().build(); + final Response.ResponseBuilder responseBuilder = Response.ok(); + + if (contentDispositionHeaderValue != null) { + responseBuilder.header(CONTENT_DISPOSITION_RESPONSE_HEADER, contentDispositionHeaderValue); + } + + return responseBuilder.build(); } ResultFormat preferredFormat = getPreferredResultFormat(resultFormat, msqControllerTask.getQuerySpec()); - return Response.ok((StreamingOutput) outputStream -> resultPusher( + final Response.ResponseBuilder responseBuilder = Response.ok((StreamingOutput) outputStream -> resultPusher( queryId, signature, closer, results, new CountingOutputStream(outputStream), preferredFormat - )).build(); + )); + + if (contentDispositionHeaderValue != null) { + responseBuilder.header(CONTENT_DISPOSITION_RESPONSE_HEADER, contentDispositionHeaderValue); + } + + return responseBuilder.build(); } diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlMSQStatementResourcePostTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlMSQStatementResourcePostTest.java index e810de56f4e1..0437fbccda25 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlMSQStatementResourcePostTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlMSQStatementResourcePostTest.java @@ -393,6 +393,7 @@ public void testWithDurableStorage() throws IOException sqlStatementResult.getQueryId(), null, ResultFormat.OBJECTLINES.name(), + null, SqlStatementResourceTest.makeOkRequest() ), objectMapper @@ -406,6 +407,7 @@ public void testWithDurableStorage() throws IOException sqlStatementResult.getQueryId(), 0L, ResultFormat.OBJECTLINES.name(), + null, SqlStatementResourceTest.makeOkRequest() ), objectMapper @@ -419,6 +421,7 @@ public void testWithDurableStorage() throws IOException sqlStatementResult.getQueryId(), 2L, ResultFormat.OBJECTLINES.name(), + null, SqlStatementResourceTest.makeOkRequest() ), objectMapper @@ -485,6 +488,7 @@ public void testMultipleWorkersWithPageSizeLimiting() throws IOException sqlStatementResult.getQueryId(), null, ResultFormat.ARRAY.name(), + null, SqlStatementResourceTest.makeOkRequest() ))); @@ -492,6 +496,7 @@ public void testMultipleWorkersWithPageSizeLimiting() throws IOException sqlStatementResult.getQueryId(), 0L, ResultFormat.ARRAY.name(), + null, SqlStatementResourceTest.makeOkRequest() ))); @@ -499,6 +504,7 @@ public void testMultipleWorkersWithPageSizeLimiting() throws IOException sqlStatementResult.getQueryId(), 1L, ResultFormat.ARRAY.name(), + null, SqlStatementResourceTest.makeOkRequest() ))); @@ -506,6 +512,7 @@ public void testMultipleWorkersWithPageSizeLimiting() throws IOException sqlStatementResult.getQueryId(), 2L, ResultFormat.ARRAY.name(), + null, SqlStatementResourceTest.makeOkRequest() ))); @@ -513,6 +520,7 @@ public void testMultipleWorkersWithPageSizeLimiting() throws IOException sqlStatementResult.getQueryId(), 3L, ResultFormat.ARRAY.name(), + null, SqlStatementResourceTest.makeOkRequest() ))); @@ -520,6 +528,7 @@ public void testMultipleWorkersWithPageSizeLimiting() throws IOException sqlStatementResult.getQueryId(), 4L, ResultFormat.ARRAY.name(), + null, SqlStatementResourceTest.makeOkRequest() ))); } @@ -565,6 +574,7 @@ public void testResultFormat() throws Exception sqlStatementResult.getQueryId(), null, resultFormat.name(), + null, SqlStatementResourceTest.makeOkRequest() ), objectMapper ) @@ -577,6 +587,7 @@ public void testResultFormat() throws Exception sqlStatementResult.getQueryId(), 0L, resultFormat.name(), + null, SqlStatementResourceTest.makeOkRequest() ), objectMapper ) @@ -616,6 +627,7 @@ public void testResultFormatWithParamInSelect() throws IOException sqlStatementResult.getQueryId(), null, ResultFormat.ARRAY.name(), + null, SqlStatementResourceTest.makeOkRequest() ))); @@ -623,6 +635,7 @@ public void testResultFormatWithParamInSelect() throws IOException sqlStatementResult.getQueryId(), 0L, ResultFormat.ARRAY.name(), + null, SqlStatementResourceTest.makeOkRequest() ))); } @@ -695,6 +708,7 @@ public void testInsert() actual.getQueryId(), 0L, null, + null, SqlStatementResourceTest.makeOkRequest() ); Assert.assertEquals(Response.Status.OK.getStatusCode(), resultsResponse.getStatus()); @@ -738,6 +752,7 @@ public void testReplaceAll() actual.getQueryId(), 0L, null, + null, SqlStatementResourceTest.makeOkRequest() ); Assert.assertEquals(Response.Status.OK.getStatusCode(), resultsResponse.getStatus()); diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java index 40dcb303b1dc..64ba45a979ba 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.SettableFuture; import org.apache.calcite.sql.type.SqlTypeName; @@ -92,6 +93,7 @@ import org.mockito.Mock; import org.mockito.Mockito; +import javax.annotation.Nullable; import javax.ws.rs.core.Response; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -683,6 +685,16 @@ private static AuthenticationResult makeAuthResultForUser(String user) ); } + @Nullable + private Object getHeader(Response resp, String header) + { + final List objects = resp.getMetadata().get(header); + if (objects == null) { + return null; + } + return Iterables.getOnlyElement(objects); + } + @BeforeEach public void init() { @@ -716,7 +728,7 @@ public void testMSQSelectAcceptedQuery() ); assertExceptionMessage( - resource.doGetResults(ACCEPTED_SELECT_MSQ_QUERY, 0L, null, makeOkRequest()), + resource.doGetResults(ACCEPTED_SELECT_MSQ_QUERY, 0L, null, null, makeOkRequest()), StringUtils.format( "Query[%s] is currently in [%s] state. Please wait for it to complete.", ACCEPTED_SELECT_MSQ_QUERY, @@ -750,7 +762,7 @@ public void testMSQSelectRunningQuery() ); assertExceptionMessage( - resource.doGetResults(RUNNING_SELECT_MSQ_QUERY, 0L, null, makeOkRequest()), + resource.doGetResults(RUNNING_SELECT_MSQ_QUERY, 0L, null, null, makeOkRequest()), StringUtils.format( "Query[%s] is currently in [%s] state. Please wait for it to complete.", RUNNING_SELECT_MSQ_QUERY, @@ -816,7 +828,7 @@ public void testFinishedSelectMSQQuery() throws Exception null )), objectMapper.writeValueAsString(response.getEntity())); - Response resultsResponse = resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), makeOkRequest()); + Response resultsResponse = resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), null, makeOkRequest()); Assert.assertEquals(Response.Status.OK.getStatusCode(), resultsResponse.getStatus()); String expectedResult = "{\"_time\":123,\"alias\":\"foo\",\"market\":\"bar\"}\n" @@ -824,6 +836,8 @@ public void testFinishedSelectMSQQuery() throws Exception assertExpectedResults(expectedResult, resultsResponse); + Assert.assertNull(getHeader(resultsResponse, SqlStatementResource.CONTENT_DISPOSITION_RESPONSE_HEADER)); + Assert.assertEquals( Response.Status.OK.getStatusCode(), resource.deleteQuery(FINISHED_SELECT_MSQ_QUERY, makeOkRequest()).getStatus() @@ -835,6 +849,7 @@ public void testFinishedSelectMSQQuery() throws Exception FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), + null, makeOkRequest() ) ); @@ -845,13 +860,22 @@ public void testFinishedSelectMSQQuery() throws Exception FINISHED_SELECT_MSQ_QUERY, null, ResultFormat.OBJECTLINES.name(), + null, makeOkRequest() ) ); Assert.assertEquals( Response.Status.BAD_REQUEST.getStatusCode(), - resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, -1L, null, makeOkRequest()).getStatus() + resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, -1L, null, null, makeOkRequest()).getStatus() + ); + + Assert.assertEquals( + "attachment; filename=my-file.ndjson", + getHeader( + resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), "my-file.ndjson", makeOkRequest()), + SqlStatementResource.CONTENT_DISPOSITION_RESPONSE_HEADER + ) ); } @@ -867,7 +891,7 @@ public void testFailedMSQQuery() for (String queryID : ImmutableList.of(ERRORED_SELECT_MSQ_QUERY, ERRORED_INSERT_MSQ_QUERY)) { assertExceptionMessage(resource.doGetStatus(queryID, false, makeOkRequest()), FAILURE_MSG, Response.Status.OK); assertExceptionMessage( - resource.doGetResults(queryID, 0L, null, makeOkRequest()), + resource.doGetResults(queryID, 0L, null, null, makeOkRequest()), StringUtils.format( "Query[%s] failed. Check the status api for more details.", queryID @@ -897,18 +921,29 @@ public void testFinishedInsertMSQQuery() null ), (SqlStatementResult) response.getEntity()); + final Response resultResponse = resource.doGetResults(FINISHED_INSERT_MSQ_QUERY, 0L, null, null, makeOkRequest()); + Assert.assertEquals( Response.Status.OK.getStatusCode(), - resource.doGetResults(FINISHED_INSERT_MSQ_QUERY, 0L, null, makeOkRequest()).getStatus() + resultResponse.getStatus() ); + Assert.assertNull(getHeader(resultResponse, SqlStatementResource.CONTENT_DISPOSITION_RESPONSE_HEADER)); Assert.assertEquals( Response.Status.OK.getStatusCode(), - resource.doGetResults(FINISHED_INSERT_MSQ_QUERY, null, null, makeOkRequest()).getStatus() + resource.doGetResults(FINISHED_INSERT_MSQ_QUERY, null, null, null, makeOkRequest()).getStatus() ); Assert.assertEquals( Response.Status.BAD_REQUEST.getStatusCode(), - resource.doGetResults(FINISHED_INSERT_MSQ_QUERY, -1L, null, makeOkRequest()).getStatus() + resource.doGetResults(FINISHED_INSERT_MSQ_QUERY, -1L, null, null, makeOkRequest()).getStatus() + ); + + Assert.assertEquals( + "attachment; filename=my-file.ndjson", + getHeader( + resource.doGetResults(FINISHED_INSERT_MSQ_QUERY, 0L, null, "my-file.ndjson", makeOkRequest()), + SqlStatementResource.CONTENT_DISPOSITION_RESPONSE_HEADER + ) ); } @@ -917,7 +952,7 @@ public void testNonMSQTasks() { for (String queryID : ImmutableList.of(RUNNING_NON_MSQ_TASK, FAILED_NON_MSQ_TASK, FINISHED_NON_MSQ_TASK)) { assertNotFound(resource.doGetStatus(queryID, false, makeOkRequest()), queryID); - assertNotFound(resource.doGetResults(queryID, 0L, null, makeOkRequest()), queryID); + assertNotFound(resource.doGetResults(queryID, 0L, null, null, makeOkRequest()), queryID); assertNotFound(resource.deleteQuery(queryID, makeOkRequest()), queryID); } } @@ -941,7 +976,7 @@ public void testMSQInsertAcceptedQuery() ); assertExceptionMessage( - resource.doGetResults(ACCEPTED_INSERT_MSQ_TASK, 0L, null, makeOkRequest()), + resource.doGetResults(ACCEPTED_INSERT_MSQ_TASK, 0L, null, null, makeOkRequest()), StringUtils.format( "Query[%s] is currently in [%s] state. Please wait for it to complete.", ACCEPTED_INSERT_MSQ_TASK, @@ -974,7 +1009,7 @@ public void testMSQInsertRunningQuery() ); assertExceptionMessage( - resource.doGetResults(RUNNING_INSERT_MSQ_QUERY, 0L, null, makeOkRequest()), + resource.doGetResults(RUNNING_INSERT_MSQ_QUERY, 0L, null, null, makeOkRequest()), StringUtils.format( "Query[%s] is currently in [%s] state. Please wait for it to complete.", RUNNING_INSERT_MSQ_QUERY, @@ -1005,6 +1040,7 @@ public void testAPIBehaviourWithSuperUsers() RUNNING_SELECT_MSQ_QUERY, 1L, null, + null, makeExpectedReq(makeAuthResultForUser(SUPERUSER)) ).getStatus() ); @@ -1035,6 +1071,7 @@ public void testAPIBehaviourWithDifferentUserAndNoStatePermission() RUNNING_SELECT_MSQ_QUERY, 1L, null, + null, makeExpectedReq(differentUserAuthResult) ).getStatus() ); @@ -1065,6 +1102,7 @@ public void testAPIBehaviourWithDifferentUserAndStateRPermission() RUNNING_SELECT_MSQ_QUERY, 1L, null, + null, makeExpectedReq(differentUserAuthResult) ).getStatus() ); @@ -1095,6 +1133,7 @@ public void testAPIBehaviourWithDifferentUserAndStateWPermission() RUNNING_SELECT_MSQ_QUERY, 1L, null, + null, makeExpectedReq(differentUserAuthResult) ).getStatus() ); @@ -1125,6 +1164,7 @@ public void testAPIBehaviourWithDifferentUserAndStateRWPermission() RUNNING_SELECT_MSQ_QUERY, 1L, null, + null, makeExpectedReq(differentUserAuthResult) ).getStatus() ); @@ -1156,7 +1196,7 @@ public void testTaskIdNotFound() ); Assert.assertEquals( Response.Status.NOT_FOUND.getStatusCode(), - resource.doGetResults(taskIdNotFound, null, null, makeOkRequest()).getStatus() + resource.doGetResults(taskIdNotFound, null, null, null, makeOkRequest()).getStatus() ); Assert.assertEquals( Response.Status.NOT_FOUND.getStatusCode(), From 6596c7f426b967c2569982a328fde8f5f593aacb Mon Sep 17 00:00:00 2001 From: John Gozde Date: Fri, 21 Mar 2025 16:10:38 -0600 Subject: [PATCH 02/10] Use StringUtils.format --- .../apache/druid/msq/sql/resources/SqlStatementResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java index 1d7a3c3f2b3c..eac08c953a54 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java @@ -311,7 +311,7 @@ public Response doGetResults( ); throwIfQueryIsNotSuccessful(queryId, statusPlus); - final String contentDispositionHeaderValue = filename != null ? String.format("attachment; filename=%s", filename) : null; + final String contentDispositionHeaderValue = filename != null ? StringUtils.format("attachment; filename=%s", filename) : null; Optional> signature = SqlStatementResourceHelper.getSignature(msqControllerTask); if (!signature.isPresent() || MSQControllerTask.isIngestion(msqControllerTask.getQuerySpec())) { From 7ae5d77afd62a2d92041bd66a7c49344089ae86b Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Thu, 27 Mar 2025 15:24:32 +0530 Subject: [PATCH 03/10] Update docs --- docs/api-reference/sql-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api-reference/sql-api.md b/docs/api-reference/sql-api.md index cead5f6040d1..669ed9297063 100644 --- a/docs/api-reference/sql-api.md +++ b/docs/api-reference/sql-api.md @@ -1288,7 +1288,7 @@ Getting the query results for an ingestion query returns an empty response. * Defines the format in which the results are presented. The following options are supported `arrayLines`,`objectLines`,`array`,`object`, and `csv`. The default is `object`. * `filename` (optional) * Type: String - * If set, attaches a `Content-Disposition` header to the response with the value of `attachment; filename={filename}`. + * If set, attaches a `Content-Disposition` header to the response with the value of `attachment; filename={filename}`. The filename must be not longer than 255 characters and must not contain the characters `/`, `\`, `:`, `*`, `?`, `"`, `<`, `>`, `|`, `\0`, `\n`, or `\r`. #### Responses From 8546fb7fd18ff223ebd0c6ac5a5fc789b685d628 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Thu, 27 Mar 2025 15:37:29 +0530 Subject: [PATCH 04/10] Add validation --- .../sql/resources/SqlStatementResource.java | 63 +++++++++++++------ .../resources/SqlStatementResourceTest.java | 44 +++++++++++++ 2 files changed, 87 insertions(+), 20 deletions(-) diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java index eac08c953a54..4e229f5ae225 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java @@ -95,6 +95,7 @@ import org.jboss.netty.handler.codec.http.HttpResponseStatus; import javax.servlet.http.HttpServletRequest; +import javax.validation.constraints.NotNull; import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.GET; @@ -114,6 +115,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -123,6 +125,7 @@ public class SqlStatementResource public static final String RESULT_FORMAT = "__resultFormat"; public static final String CONTENT_DISPOSITION_RESPONSE_HEADER = "Content-Disposition"; + private static final Pattern FILENAME_PATTERN = Pattern.compile("^[^/:*?><\\\\\"|\0\n\r]*$"); private static final Logger log = new Logger(SqlStatementResource.class); private final SqlStatementFactory msqSqlStatementFactory; private final ObjectMapper jsonMapper; @@ -311,18 +314,12 @@ public Response doGetResults( ); throwIfQueryIsNotSuccessful(queryId, statusPlus); - final String contentDispositionHeaderValue = filename != null ? StringUtils.format("attachment; filename=%s", filename) : null; + final String contentDispositionHeaderValue = filename != null ? StringUtils.format("attachment; filename=\"%s\"", validateFilename(filename)) : null; Optional> signature = SqlStatementResourceHelper.getSignature(msqControllerTask); if (!signature.isPresent() || MSQControllerTask.isIngestion(msqControllerTask.getQuerySpec())) { // Since it's not a select query, nothing to return. - final Response.ResponseBuilder responseBuilder = Response.ok(); - - if (contentDispositionHeaderValue != null) { - responseBuilder.header(CONTENT_DISPOSITION_RESPONSE_HEADER, contentDispositionHeaderValue); - } - - return responseBuilder.build(); + return addContentDisposition(Response.ok(), contentDispositionHeaderValue).build(); } // returning results @@ -331,13 +328,7 @@ public Response doGetResults( results = getResultYielder(queryId, page, msqControllerTask, closer); if (!results.isPresent()) { // no results, return empty - final Response.ResponseBuilder responseBuilder = Response.ok(); - - if (contentDispositionHeaderValue != null) { - responseBuilder.header(CONTENT_DISPOSITION_RESPONSE_HEADER, contentDispositionHeaderValue); - } - - return responseBuilder.build(); + return addContentDisposition(Response.ok(), contentDispositionHeaderValue).build(); } ResultFormat preferredFormat = getPreferredResultFormat(resultFormat, msqControllerTask.getQuerySpec()); @@ -350,11 +341,7 @@ public Response doGetResults( preferredFormat )); - if (contentDispositionHeaderValue != null) { - responseBuilder.header(CONTENT_DISPOSITION_RESPONSE_HEADER, contentDispositionHeaderValue); - } - - return responseBuilder.build(); + return addContentDisposition(responseBuilder, contentDispositionHeaderValue).build(); } @@ -998,6 +985,42 @@ private void checkForDurableStorageConnectorImpl() } } + private static Response.ResponseBuilder addContentDisposition( + Response.ResponseBuilder responseBuilder, + String contentDisposition + ) + { + if (contentDisposition != null) { + responseBuilder.header(CONTENT_DISPOSITION_RESPONSE_HEADER, contentDisposition); + } + return responseBuilder; + } + + /** + * Validates that a filename in valid. File names are considered to be valid if it is: + *
    + *
  • Not empty.
    • + *
  • Not longer than 255 characters.
    • + *
  • Does not contain the characters `/`, `\`, `:`, `*`, `?`, `"`, `<`, `>`, `|`, `\0`, `\n`, or `\r`.
    • + *
+ */ + @VisibleForTesting + static String validateFilename(@NotNull String filename) + { + if (filename.isEmpty()) { + throw InvalidInput.exception("Filename cannot be empty."); + } + + if (filename.length() > 255) { + throw InvalidInput.exception("Filename cannot be longer than 255 characters."); + } + + if (!FILENAME_PATTERN.matcher(filename).matches()) { + throw InvalidInput.exception("Filename contains invalid characters."); + } + return filename; + } + private T contactOverlord(final ListenableFuture future, String queryId) { try { diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java index 64ba45a979ba..04a8723f8a5d 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java @@ -20,6 +20,7 @@ package org.apache.druid.msq.sql.resources; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; @@ -28,6 +29,8 @@ import org.apache.calcite.sql.type.SqlTypeName; import org.apache.druid.client.indexing.TaskPayloadResponse; import org.apache.druid.client.indexing.TaskStatusResponse; +import org.apache.druid.error.DruidException; +import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.error.ErrorResponse; import org.apache.druid.indexer.TaskLocation; import org.apache.druid.indexer.TaskState; @@ -82,6 +85,7 @@ import org.apache.druid.sql.calcite.util.CalciteTests; import org.apache.druid.sql.http.ResultFormat; import org.apache.druid.sql.http.SqlResourceTest; +import org.hamcrest.MatcherAssert; import org.jboss.netty.handler.codec.http.DefaultHttpResponse; import org.jboss.netty.handler.codec.http.HttpResponseStatus; import org.jboss.netty.handler.codec.http.HttpVersion; @@ -1233,4 +1237,44 @@ private void assertSqlStatementResult(SqlStatementResult expected, SqlStatementR Assert.assertEquals(expected.getErrorResponse().getAsMap(), actual.getErrorResponse().getAsMap()); } } + + @Test + void testValidFilename() + { + // Valid cases + SqlStatementResource.validateFilename("testname"); + SqlStatementResource.validateFilename("A.txt"); + SqlStatementResource.validateFilename("final-results.txt"); + SqlStatementResource.validateFilename("final results.txt"); + + // Empty + assertInvalidFileName("", "Filename cannot be null or empty"); + + // Too long + assertInvalidFileName(StringUtils.repeat("A", 300), "Filename cannot be longer than 255 characters"); + + // Special characters + assertInvalidFileName("He\\llo", "Filename contains invalid characters"); + assertInvalidFileName("Hello/Name", "Filename contains invalid characters"); + assertInvalidFileName("C:/Users/Name", "Filename contains invalid characters"); + assertInvalidFileName("username:password", "Filename contains invalid characters"); + assertInvalidFileName("A>B", "Filename contains invalid characters"); + assertInvalidFileName("A SqlStatementResource.validateFilename(filename)) + ), + DruidExceptionMatcher.invalidInput().expectMessageIs(errorMessage) + ); + } } From 17377e56f2d4838a7fd11fa9ccce6ba80e0c54a6 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Thu, 27 Mar 2025 16:11:40 +0530 Subject: [PATCH 05/10] Fix up tests --- .../resources/SqlStatementResourceTest.java | 55 +++++++++++++------ 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java index 04a8723f8a5d..a009e91840a6 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java @@ -875,7 +875,7 @@ public void testFinishedSelectMSQQuery() throws Exception ); Assert.assertEquals( - "attachment; filename=my-file.ndjson", + "attachment; filename=\"my-file.ndjson\"", getHeader( resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), "my-file.ndjson", makeOkRequest()), SqlStatementResource.CONTENT_DISPOSITION_RESPONSE_HEADER @@ -883,6 +883,27 @@ public void testFinishedSelectMSQQuery() throws Exception ); } + @Test + public void testDownloadResultsAsFile() throws Exception + { + final String expectedResult = "{\"_time\":123,\"alias\":\"foo\",\"market\":\"bar\"}\n" + + "{\"_time\":234,\"alias\":\"foo1\",\"market\":\"bar1\"}\n\n"; + + Response resultsResponse1 = resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), "results.txt", makeOkRequest()); + Assert.assertEquals(Response.Status.OK.getStatusCode(), resultsResponse1.getStatus()); + Assert.assertEquals("attachment; filename=\"results.txt\"", resultsResponse1.getMetadata().get("Content-Disposition").get(0)); + assertExpectedResults(expectedResult, resultsResponse1); + + Response resultsResponse2 = resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), "final results.txt", makeOkRequest()); + Assert.assertEquals(Response.Status.OK.getStatusCode(), resultsResponse2.getStatus()); + Assert.assertEquals("attachment; filename=\"final results.txt\"", resultsResponse2.getMetadata().get("Content-Disposition").get(0)); + assertExpectedResults(expectedResult, resultsResponse2); + + Response resultsResponse3 = resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), "/Users/Name/final.txt", makeOkRequest()); + Assert.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), resultsResponse3.getStatus()); + Assert.assertNull(resultsResponse3.getMetadata().get("Content-Disposition")); + } + private void assertExpectedResults(String expectedResult, Response resultsResponse) throws IOException { byte[] bytes = SqlResourceTest.responseToByteArray(resultsResponse); @@ -943,7 +964,7 @@ public void testFinishedInsertMSQQuery() ); Assert.assertEquals( - "attachment; filename=my-file.ndjson", + "attachment; filename=\"my-file.ndjson\"", getHeader( resource.doGetResults(FINISHED_INSERT_MSQ_QUERY, 0L, null, "my-file.ndjson", makeOkRequest()), SqlStatementResource.CONTENT_DISPOSITION_RESPONSE_HEADER @@ -1246,26 +1267,28 @@ void testValidFilename() SqlStatementResource.validateFilename("A.txt"); SqlStatementResource.validateFilename("final-results.txt"); SqlStatementResource.validateFilename("final results.txt"); + SqlStatementResource.validateFilename("final;results.txt"); + SqlStatementResource.validateFilename("final@results.txt"); // Empty - assertInvalidFileName("", "Filename cannot be null or empty"); + assertInvalidFileName("", "Filename cannot be empty."); // Too long - assertInvalidFileName(StringUtils.repeat("A", 300), "Filename cannot be longer than 255 characters"); + assertInvalidFileName(StringUtils.repeat("A", 300), "Filename cannot be longer than 255 characters."); // Special characters - assertInvalidFileName("He\\llo", "Filename contains invalid characters"); - assertInvalidFileName("Hello/Name", "Filename contains invalid characters"); - assertInvalidFileName("C:/Users/Name", "Filename contains invalid characters"); - assertInvalidFileName("username:password", "Filename contains invalid characters"); - assertInvalidFileName("A>B", "Filename contains invalid characters"); - assertInvalidFileName("AB", "Filename contains invalid characters."); + assertInvalidFileName("A Date: Thu, 27 Mar 2025 16:20:09 +0530 Subject: [PATCH 06/10] Update docs --- docs/api-reference/sql-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api-reference/sql-api.md b/docs/api-reference/sql-api.md index 669ed9297063..ccba5b154355 100644 --- a/docs/api-reference/sql-api.md +++ b/docs/api-reference/sql-api.md @@ -1288,7 +1288,7 @@ Getting the query results for an ingestion query returns an empty response. * Defines the format in which the results are presented. The following options are supported `arrayLines`,`objectLines`,`array`,`object`, and `csv`. The default is `object`. * `filename` (optional) * Type: String - * If set, attaches a `Content-Disposition` header to the response with the value of `attachment; filename={filename}`. The filename must be not longer than 255 characters and must not contain the characters `/`, `\`, `:`, `*`, `?`, `"`, `<`, `>`, `|`, `\0`, `\n`, or `\r`. + * If set, attaches a `Content-Disposition` header to the response with the value of `attachment; filename={filename}`. The filename must not be longer than 255 characters and must not contain the characters `/`, `\`, `:`, `*`, `?`, `"`, `<`, `>`, `|`, `\0`, `\n`, or `\r`. #### Responses From 2ccd0941c0e7ee62d7fc93c69510e09ed7852bc7 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Thu, 27 Mar 2025 18:30:10 +0530 Subject: [PATCH 07/10] Rewrite patch --- .../druid/msq/sql/resources/SqlStatementResourceTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java index a009e91840a6..66ca4cbd94a4 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java @@ -85,7 +85,6 @@ import org.apache.druid.sql.calcite.util.CalciteTests; import org.apache.druid.sql.http.ResultFormat; import org.apache.druid.sql.http.SqlResourceTest; -import org.hamcrest.MatcherAssert; import org.jboss.netty.handler.codec.http.DefaultHttpResponse; import org.jboss.netty.handler.codec.http.HttpResponseStatus; import org.jboss.netty.handler.codec.http.HttpVersion; @@ -108,6 +107,8 @@ import java.util.List; import java.util.function.Supplier; +import static org.hamcrest.MatcherAssert.assertThat; + public class SqlStatementResourceTest extends MSQTestBase { public static final DateTime CREATED_TIME = DateTimes.of("2023-05-31T12:00Z"); @@ -1293,7 +1294,7 @@ void testValidFilename() private void assertInvalidFileName(String filename, String errorMessage) { - MatcherAssert.assertThat( + assertThat( Throwables.getRootCause( Assert.assertThrows(DruidException.class, () -> SqlStatementResource.validateFilename(filename)) ), From 60feb4b67aa8b5597407b996e273fd7773963934 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Thu, 27 Mar 2025 18:31:43 +0530 Subject: [PATCH 08/10] Clean up tests --- .../sql/resources/SqlStatementResourceTest.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java index 66ca4cbd94a4..bb303fb5b243 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java @@ -20,7 +20,6 @@ package org.apache.druid.msq.sql.resources; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; @@ -892,12 +891,18 @@ public void testDownloadResultsAsFile() throws Exception Response resultsResponse1 = resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), "results.txt", makeOkRequest()); Assert.assertEquals(Response.Status.OK.getStatusCode(), resultsResponse1.getStatus()); - Assert.assertEquals("attachment; filename=\"results.txt\"", resultsResponse1.getMetadata().get("Content-Disposition").get(0)); + Assert.assertEquals( + "attachment; filename=\"results.txt\"", + getHeader(resultsResponse1, "Content-Disposition") + ); assertExpectedResults(expectedResult, resultsResponse1); Response resultsResponse2 = resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), "final results.txt", makeOkRequest()); Assert.assertEquals(Response.Status.OK.getStatusCode(), resultsResponse2.getStatus()); - Assert.assertEquals("attachment; filename=\"final results.txt\"", resultsResponse2.getMetadata().get("Content-Disposition").get(0)); + Assert.assertEquals( + "attachment; filename=\"final results.txt\"", + getHeader(resultsResponse2, "Content-Disposition") + ); assertExpectedResults(expectedResult, resultsResponse2); Response resultsResponse3 = resource.doGetResults(FINISHED_SELECT_MSQ_QUERY, 0L, ResultFormat.OBJECTLINES.name(), "/Users/Name/final.txt", makeOkRequest()); @@ -1261,7 +1266,7 @@ private void assertSqlStatementResult(SqlStatementResult expected, SqlStatementR } @Test - void testValidFilename() + public void testValidFilename() { // Valid cases SqlStatementResource.validateFilename("testname"); @@ -1295,9 +1300,7 @@ void testValidFilename() private void assertInvalidFileName(String filename, String errorMessage) { assertThat( - Throwables.getRootCause( - Assert.assertThrows(DruidException.class, () -> SqlStatementResource.validateFilename(filename)) - ), + Assert.assertThrows(DruidException.class, () -> SqlStatementResource.validateFilename(filename)), DruidExceptionMatcher.invalidInput().expectMessageIs(errorMessage) ); } From ebf084127c05be4a331d26aa7554d2807fff48f7 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Fri, 28 Mar 2025 10:47:16 +0530 Subject: [PATCH 09/10] Update error message --- .../sql/resources/SqlStatementResource.java | 4 ++-- .../resources/SqlStatementResourceTest.java | 24 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java index 4e229f5ae225..f0a800c246ab 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java @@ -997,7 +997,7 @@ private static Response.ResponseBuilder addContentDisposition( } /** - * Validates that a filename in valid. File names are considered to be valid if it is: + * Validates that a filename is valid. Filenames are considered to be valid if it is: *
    *
  • Not empty.
    • *
  • Not longer than 255 characters.
    • @@ -1016,7 +1016,7 @@ static String validateFilename(@NotNull String filename) } if (!FILENAME_PATTERN.matcher(filename).matches()) { - throw InvalidInput.exception("Filename contains invalid characters."); + throw InvalidInput.exception("Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); } return filename; } diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java index bb303fb5b243..b2188e2ebc5c 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java @@ -1283,18 +1283,18 @@ public void testValidFilename() assertInvalidFileName(StringUtils.repeat("A", 300), "Filename cannot be longer than 255 characters."); // Special characters - assertInvalidFileName("He\\llo", "Filename contains invalid characters."); - assertInvalidFileName("Hello/Name", "Filename contains invalid characters."); - assertInvalidFileName("C:/Users/Name", "Filename contains invalid characters."); - assertInvalidFileName("username:password", "Filename contains invalid characters."); - assertInvalidFileName("A>B", "Filename contains invalid characters."); - assertInvalidFileName("A, |, \\0, \\n, or \\r)"); + assertInvalidFileName("Hello/Name", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("C:/Users/Name", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("username:password", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("A>ValueB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("A, |, \\0, \\n, or \\r)"); + assertInvalidFileName("A\0a11", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("\rrB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("\nAnB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("A|B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("A\"B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("A?B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); } private void assertInvalidFileName(String filename, String errorMessage) From 3a36d58c7a1b41e2697169e59fe1bd9eceb395ef Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Fri, 28 Mar 2025 12:12:12 +0530 Subject: [PATCH 10/10] Fix tests --- .../resources/SqlStatementResourceTest.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java index b2188e2ebc5c..83ae1de7f9d3 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/resources/SqlStatementResourceTest.java @@ -1283,18 +1283,18 @@ public void testValidFilename() assertInvalidFileName(StringUtils.repeat("A", 300), "Filename cannot be longer than 255 characters."); // Special characters - assertInvalidFileName("He\\llo", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("Hello/Name", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("C:/Users/Name", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("username:password", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("A>ValueB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("A, |, \\0, \\n, or \\r)"); - assertInvalidFileName("A\0a11", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("\rrB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("\nAnB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("A|B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("A\"B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); - assertInvalidFileName("A?B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \\0, \\n, or \\r)"); + assertInvalidFileName("He\\llo", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("Hello/Name", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("C:/Users/Name", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("username:password", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("A>ValueB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("A, |, \0, \n, or \r)"); + assertInvalidFileName("A\0a11", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("\rrB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("\nAnB", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("A|B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("A\"B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); + assertInvalidFileName("A?B", "Filename contains invalid characters. (/, \\, :, *, ?, \", <, >, |, \0, \n, or \r)"); } private void assertInvalidFileName(String filename, String errorMessage)