This is an automated email from the ASF dual-hosted git repository.
maytasm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new a04b08e Add new config to filter internal Druid-related messages from
Query API response (#11711)
a04b08e is described below
commit a04b08e45ce82c6c27eca94b735e624349846bc5
Author: Maytas Monsereenusorn <[email protected]>
AuthorDate: Wed Sep 29 12:55:49 2021 +0700
Add new config to filter internal Druid-related messages from Query API
response (#11711)
* add impl
* add impl
* add tests
* add unit test
* fix checkstyle
* address comments
* fix checkstyle
* fix checkstyle
* fix checkstyle
* fix checkstyle
* fix checkstyle
* address comments
* address comments
* address comments
* fix test
* fix test
* fix test
* fix test
* fix test
* change config name
* change config name
* change config name
* address comments
* address comments
* address comments
* address comments
* address comments
* address comments
* fix compile
* fix compile
* change config
* add more tests
* fix IT
---
...AllowedRegexErrorResponseTransformStrategy.java | 80 +++++++
.../exception/ErrorResponseTransformStrategy.java | 50 ++++
.../NoErrorResponseTransformStrategy.java | 57 +++++
.../common/exception/SanitizableException.java | 37 +++
.../org/apache/druid/query/QueryException.java | 11 +-
...wedRegexErrorResponseTransformStrategyTest.java | 70 ++++++
.../NoErrorResponseTransformStrategyTest.java | 31 +--
.../org/apache/druid/query/QueryExceptionTest.java | 73 ++++++
.../org/apache/druid/tests/security/ITTLSTest.java | 2 +-
.../druid/server/initialization/ServerConfig.java | 39 ++-
.../jetty/CliIndexerServerModule.java | 4 +-
.../initialization/jetty/JettyServerModule.java | 29 +++
.../druid/server/security/ForbiddenException.java | 21 +-
.../druid/initialization/ServerConfigTest.java | 6 +-
.../druid/server/initialization/BaseJettyTest.java | 2 +-
.../druid/server/initialization/JettyTest.java | 7 +
.../JettyWithResponseFilterEnabledTest.java} | 32 +--
.../server/security/ForbiddenExceptionTest.java | 64 +++++
.../druid/server/AsyncQueryForwardingServlet.java | 28 ++-
.../server/AsyncQueryForwardingServletTest.java | 262 ++++++++++++++++++++-
.../org/apache/druid/sql/http/SqlResource.java | 17 +-
.../org/apache/druid/sql/http/SqlResourceTest.java | 93 ++++++--
22 files changed, 921 insertions(+), 94 deletions(-)
diff --git
a/core/src/main/java/org/apache/druid/common/exception/AllowedRegexErrorResponseTransformStrategy.java
b/core/src/main/java/org/apache/druid/common/exception/AllowedRegexErrorResponseTransformStrategy.java
new file mode 100644
index 0000000..dde76bd
--- /dev/null
+++
b/core/src/main/java/org/apache/druid/common/exception/AllowedRegexErrorResponseTransformStrategy.java
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.common.exception;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.collect.ImmutableList;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.function.Function;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+public class AllowedRegexErrorResponseTransformStrategy implements
ErrorResponseTransformStrategy
+{
+ @JsonProperty
+ private final List<String> allowedRegexString;
+
+ private final List<Pattern> allowedRegexPattern;
+
+ @JsonCreator
+ public AllowedRegexErrorResponseTransformStrategy(
+ @JsonProperty("allowedRegex") List<String> allowedRegexString
+ )
+ {
+ this.allowedRegexString = allowedRegexString;
+ this.allowedRegexPattern = allowedRegexString == null
+ ? ImmutableList.of()
+ :
allowedRegexString.stream().map(Pattern::compile).collect(Collectors.toList());
+ }
+
+ @Override
+ public Function<String, String> getErrorMessageTransformFunction()
+ {
+ return (String errorMessage) -> {
+ if (allowedRegexPattern.stream().anyMatch(pattern ->
pattern.matcher(errorMessage).matches())) {
+ return errorMessage;
+ } else {
+ return null;
+ }
+ };
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ AllowedRegexErrorResponseTransformStrategy that =
(AllowedRegexErrorResponseTransformStrategy) o;
+ return Objects.equals(allowedRegexString, that.allowedRegexString);
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(allowedRegexString);
+ }
+}
diff --git
a/core/src/main/java/org/apache/druid/common/exception/ErrorResponseTransformStrategy.java
b/core/src/main/java/org/apache/druid/common/exception/ErrorResponseTransformStrategy.java
new file mode 100644
index 0000000..d7a2bbd
--- /dev/null
+++
b/core/src/main/java/org/apache/druid/common/exception/ErrorResponseTransformStrategy.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.common.exception;
+
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+
+import javax.validation.constraints.NotNull;
+import java.util.function.Function;
+
+@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "strategy", defaultImpl =
NoErrorResponseTransformStrategy.class)
+@JsonSubTypes(value = {
+ @JsonSubTypes.Type(name = "none", value =
NoErrorResponseTransformStrategy.class),
+ @JsonSubTypes.Type(name = "allowedRegex", value =
AllowedRegexErrorResponseTransformStrategy.class)
+})
+public interface ErrorResponseTransformStrategy
+{
+ /**
+ * For a given {@link SanitizableException} apply the transformation
strategy and return the sanitized Exception
+ * if the transformation stategy was applied.
+ */
+ default Exception transformIfNeeded(SanitizableException exception)
+ {
+ return exception.sanitize(getErrorMessageTransformFunction());
+ }
+
+ /**
+ * Return a function for checking and transforming the error message if
needed.
+ * Function can return null if error message needs to be omitted or return
String to be use instead.
+ */
+ @NotNull
+ Function<String, String> getErrorMessageTransformFunction();
+}
diff --git
a/core/src/main/java/org/apache/druid/common/exception/NoErrorResponseTransformStrategy.java
b/core/src/main/java/org/apache/druid/common/exception/NoErrorResponseTransformStrategy.java
new file mode 100644
index 0000000..95ebb8b
--- /dev/null
+++
b/core/src/main/java/org/apache/druid/common/exception/NoErrorResponseTransformStrategy.java
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.common.exception;
+
+import java.util.function.Function;
+
+/**
+ * Error response transform strategy that does nothing and simply return the
same Exception back without any change
+ */
+public class NoErrorResponseTransformStrategy implements
ErrorResponseTransformStrategy
+{
+ public static final NoErrorResponseTransformStrategy INSTANCE = new
NoErrorResponseTransformStrategy();
+
+ @Override
+ public Exception transformIfNeeded(SanitizableException exception)
+ {
+ return (Exception) exception;
+ }
+
+ @Override
+ public Function<String, String> getErrorMessageTransformFunction()
+ {
+ return (String errorMessage) -> errorMessage;
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ return !(o == null || getClass() != o.getClass());
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return NoErrorResponseTransformStrategy.class.hashCode();
+ }
+}
diff --git
a/core/src/main/java/org/apache/druid/common/exception/SanitizableException.java
b/core/src/main/java/org/apache/druid/common/exception/SanitizableException.java
new file mode 100644
index 0000000..2ac15e0
--- /dev/null
+++
b/core/src/main/java/org/apache/druid/common/exception/SanitizableException.java
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.common.exception;
+
+import java.util.function.Function;
+
+public interface SanitizableException
+{
+ /**
+ * Apply the function for transforming the error message then return new
Exception with sanitized fields and transformed message.
+ * The {@param errorMessageTransformFunction} is only intended to be use to
transform the error message
+ * String of the Exception as only the error message String is common to all
Exception classes.
+ * For other fields (which may be unique to each particular Exception
class), each implementation of this method can
+ * decide for itself how to sanitized those fields (i.e. leaving unchanged,
changing to null, changing to a fixed String, etc.).
+ * Note that this method returns a new Exception of the same type since
Exception error message is immutable.
+ */
+ Exception sanitize(
+ Function<String, String> errorMessageTransformFunction
+ );
+}
diff --git a/core/src/main/java/org/apache/druid/query/QueryException.java
b/core/src/main/java/org/apache/druid/query/QueryException.java
index 108e9f7..10267b5 100644
--- a/core/src/main/java/org/apache/druid/query/QueryException.java
+++ b/core/src/main/java/org/apache/druid/query/QueryException.java
@@ -22,16 +22,19 @@ package org.apache.druid.query;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.annotations.VisibleForTesting;
+import org.apache.druid.common.exception.SanitizableException;
import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
import java.net.InetAddress;
+import java.util.function.Function;
/**
* Base serializable error response
*
* QueryResource and SqlResource are expected to emit the JSON form of this
object when errors happen.
*/
-public class QueryException extends RuntimeException
+public class QueryException extends RuntimeException implements
SanitizableException
{
private final String errorCode;
private final String errorClass;
@@ -96,4 +99,10 @@ public class QueryException extends RuntimeException
return null;
}
}
+
+ @Override
+ public QueryException sanitize(@NotNull Function<String, String>
errorMessageTransformFunction)
+ {
+ return new QueryException(errorCode,
errorMessageTransformFunction.apply(getMessage()), null, null);
+ }
}
diff --git
a/core/src/test/java/org/apache/druid/common/exception/AllowedRegexErrorResponseTransformStrategyTest.java
b/core/src/test/java/org/apache/druid/common/exception/AllowedRegexErrorResponseTransformStrategyTest.java
new file mode 100644
index 0000000..9ababc8
--- /dev/null
+++
b/core/src/test/java/org/apache/druid/common/exception/AllowedRegexErrorResponseTransformStrategyTest.java
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.common.exception;
+
+import com.google.common.collect.ImmutableList;
+import nl.jqno.equalsverifier.EqualsVerifier;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class AllowedRegexErrorResponseTransformStrategyTest
+{
+ @Test
+ public void
testGetErrorMessageTransformFunctionWithMatchingAllowedRegexFilter()
+ {
+ AllowedRegexErrorResponseTransformStrategy allowedRegex = new
AllowedRegexErrorResponseTransformStrategy(
+ ImmutableList.of("acbd", "test .*")
+ );
+ String message = "test message 123";
+ String result =
allowedRegex.getErrorMessageTransformFunction().apply(message);
+ Assert.assertEquals(message, result);
+ }
+
+ @Test
+ public void
testGetErrorMessageTransformFunctionWithNoMatchingAllowedRegexFilter()
+ {
+ AllowedRegexErrorResponseTransformStrategy allowedRegex = new
AllowedRegexErrorResponseTransformStrategy(
+ ImmutableList.of("acbd", "qwer")
+ );
+ String message = "test message 123";
+ String result =
allowedRegex.getErrorMessageTransformFunction().apply(message);
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void testGetErrorMessageTransformFunctionWithEmptyAllowedRegexFilter()
+ {
+ AllowedRegexErrorResponseTransformStrategy allowedRegex = new
AllowedRegexErrorResponseTransformStrategy(
+ ImmutableList.of()
+ );
+ String message = "test message 123";
+ String result =
allowedRegex.getErrorMessageTransformFunction().apply(message);
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void testEqualsAndHashCode()
+ {
+ EqualsVerifier.forClass(AllowedRegexErrorResponseTransformStrategy.class)
+ .withIgnoredFields("allowedRegexPattern")
+ .usingGetClass()
+ .verify();
+ }
+}
diff --git
a/server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
b/core/src/test/java/org/apache/druid/common/exception/NoErrorResponseTransformStrategyTest.java
similarity index 56%
copy from
server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
copy to
core/src/test/java/org/apache/druid/common/exception/NoErrorResponseTransformStrategyTest.java
index 6cf326c..8e0394c 100644
---
a/server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
+++
b/core/src/test/java/org/apache/druid/common/exception/NoErrorResponseTransformStrategyTest.java
@@ -17,31 +17,18 @@
* under the License.
*/
-package org.apache.druid.server.security;
+package org.apache.druid.common.exception;
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonProperty;
+import nl.jqno.equalsverifier.EqualsVerifier;
+import org.junit.Test;
-/**
- * Throw this when a request is unauthorized and we want to send a 403
response back, Jersey exception mapper will
- * take care of sending the response.
- */
-public class ForbiddenException extends RuntimeException
+public class NoErrorResponseTransformStrategyTest
{
- public ForbiddenException()
- {
- super("Unauthorized.");
- }
-
- @JsonCreator
- public ForbiddenException(@JsonProperty("errorMessage") String msg)
- {
- super(msg);
- }
-
- @JsonProperty
- public String getErrorMessage()
+ @Test
+ public void testEqualsAndHashCode()
{
- return super.getMessage();
+ EqualsVerifier.forClass(NoErrorResponseTransformStrategy.class)
+ .usingGetClass()
+ .verify();
}
}
diff --git a/core/src/test/java/org/apache/druid/query/QueryExceptionTest.java
b/core/src/test/java/org/apache/druid/query/QueryExceptionTest.java
new file mode 100644
index 0000000..51ff763
--- /dev/null
+++ b/core/src/test/java/org/apache/druid/query/QueryExceptionTest.java
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.function.Function;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QueryExceptionTest
+{
+ private static final String ERROR_CODE = "error code";
+ private static final String ERROR_CLASS = "error code";
+ private static final String HOST = "error code";
+ private static final String ERROR_MESSAGE_ORIGINAL = "aaaa";
+ private static final String ERROR_MESSAGE_TRANSFORMED = "bbbb";
+
+ @Mock
+ private Function<String, String> trasformFunction;
+
+ @Test
+ public void testSanitizeWithTransformFunctionReturningNull()
+ {
+
Mockito.when(trasformFunction.apply(ArgumentMatchers.eq(ERROR_MESSAGE_ORIGINAL))).thenReturn(null);
+ QueryException queryException = new QueryException(ERROR_CODE,
ERROR_MESSAGE_ORIGINAL, ERROR_CLASS, HOST);
+ QueryException actual = queryException.sanitize(trasformFunction);
+ Assert.assertNotNull(actual);
+ Assert.assertEquals(actual.getErrorCode(), ERROR_CODE);
+ Assert.assertNull(actual.getMessage());
+ Assert.assertNull(actual.getHost());
+ Assert.assertNull(actual.getErrorClass());
+
Mockito.verify(trasformFunction).apply(ArgumentMatchers.eq(ERROR_MESSAGE_ORIGINAL));
+ Mockito.verifyNoMoreInteractions(trasformFunction);
+ }
+
+ @Test
+ public void testSanitizeWithTransformFunctionReturningNewString()
+ {
+
Mockito.when(trasformFunction.apply(ArgumentMatchers.eq(ERROR_MESSAGE_ORIGINAL))).thenReturn(ERROR_MESSAGE_TRANSFORMED);
+ QueryException queryException = new QueryException(ERROR_CODE,
ERROR_MESSAGE_ORIGINAL, ERROR_CLASS, HOST);
+ QueryException actual = queryException.sanitize(trasformFunction);
+ Assert.assertNotNull(actual);
+ Assert.assertEquals(actual.getErrorCode(), ERROR_CODE);
+ Assert.assertEquals(actual.getMessage(), ERROR_MESSAGE_TRANSFORMED);
+ Assert.assertNull(actual.getHost());
+ Assert.assertNull(actual.getErrorClass());
+
Mockito.verify(trasformFunction).apply(ArgumentMatchers.eq(ERROR_MESSAGE_ORIGINAL));
+ Mockito.verifyNoMoreInteractions(trasformFunction);
+ }
+}
diff --git
a/integration-tests/src/test/java/org/apache/druid/tests/security/ITTLSTest.java
b/integration-tests/src/test/java/org/apache/druid/tests/security/ITTLSTest.java
index f889a2b..09167c0 100644
---
a/integration-tests/src/test/java/org/apache/druid/tests/security/ITTLSTest.java
+++
b/integration-tests/src/test/java/org/apache/druid/tests/security/ITTLSTest.java
@@ -258,7 +258,7 @@ public class ITTLSTest
config.getCustomCertCheckRouterTLSUrl() + "/druid/v2",
"Custom cert check",
ISE.class,
- "Error while making request to url[https://127.0.0.1:9091/druid/v2]
status[400 Bad Request] content[{\"error\":\"No content to map due to
end-of-input",
+ "Error while making request to url[https://127.0.0.1:9091/druid/v2]
status[400 Bad Request] content[{\"error\":\"Unknown
exception\",\"errorMessage\":\"No content to map due to end-of-input",
true
);
diff --git
a/server/src/main/java/org/apache/druid/server/initialization/ServerConfig.java
b/server/src/main/java/org/apache/druid/server/initialization/ServerConfig.java
index ac9576b..646c0e5 100644
---
a/server/src/main/java/org/apache/druid/server/initialization/ServerConfig.java
+++
b/server/src/main/java/org/apache/druid/server/initialization/ServerConfig.java
@@ -21,6 +21,8 @@ package org.apache.druid.server.initialization;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableList;
+import org.apache.druid.common.exception.ErrorResponseTransformStrategy;
+import org.apache.druid.common.exception.NoErrorResponseTransformStrategy;
import org.apache.druid.java.util.common.HumanReadableBytes;
import org.apache.druid.java.util.common.HumanReadableBytesRange;
import org.apache.druid.utils.JvmUtils;
@@ -61,7 +63,9 @@ public class ServerConfig
int inflateBufferSize,
int compressionLevel,
boolean enableForwardedRequestCustomizer,
- @NotNull List<String> allowedHttpMethods
+ @NotNull List<String> allowedHttpMethods,
+ boolean showDetailedJettyErrors,
+ ErrorResponseTransformStrategy errorResponseTransformStrategy
)
{
this.numThreads = numThreads;
@@ -79,6 +83,8 @@ public class ServerConfig
this.compressionLevel = compressionLevel;
this.enableForwardedRequestCustomizer = enableForwardedRequestCustomizer;
this.allowedHttpMethods = allowedHttpMethods;
+ this.showDetailedJettyErrors = showDetailedJettyErrors;
+ this.errorResponseTransformStrategy = errorResponseTransformStrategy;
}
public ServerConfig()
@@ -145,6 +151,13 @@ public class ServerConfig
@NotNull
private List<String> allowedHttpMethods = ImmutableList.of();
+ @JsonProperty("errorResponseTransform")
+ @NotNull
+ private ErrorResponseTransformStrategy errorResponseTransformStrategy =
NoErrorResponseTransformStrategy.INSTANCE;
+
+ @JsonProperty
+ private boolean showDetailedJettyErrors = true;
+
public int getNumThreads()
{
return numThreads;
@@ -215,6 +228,16 @@ public class ServerConfig
return enableForwardedRequestCustomizer;
}
+ public boolean isShowDetailedJettyErrors()
+ {
+ return showDetailedJettyErrors;
+ }
+
+ public ErrorResponseTransformStrategy getErrorResponseTransformStrategy()
+ {
+ return errorResponseTransformStrategy;
+ }
+
@NotNull
public List<String> getAllowedHttpMethods()
{
@@ -235,17 +258,19 @@ public class ServerConfig
queueSize == that.queueSize &&
enableRequestLimit == that.enableRequestLimit &&
defaultQueryTimeout == that.defaultQueryTimeout &&
- maxScatterGatherBytes.equals(that.maxScatterGatherBytes) &&
maxSubqueryRows == that.maxSubqueryRows &&
maxQueryTimeout == that.maxQueryTimeout &&
maxRequestHeaderSize == that.maxRequestHeaderSize &&
inflateBufferSize == that.inflateBufferSize &&
compressionLevel == that.compressionLevel &&
enableForwardedRequestCustomizer ==
that.enableForwardedRequestCustomizer &&
+ showDetailedJettyErrors == that.showDetailedJettyErrors &&
maxIdleTime.equals(that.maxIdleTime) &&
+ maxScatterGatherBytes.equals(that.maxScatterGatherBytes) &&
gracefulShutdownTimeout.equals(that.gracefulShutdownTimeout) &&
unannouncePropagationDelay.equals(that.unannouncePropagationDelay)
&&
- allowedHttpMethods.equals(that.allowedHttpMethods);
+ allowedHttpMethods.equals(that.allowedHttpMethods) &&
+
errorResponseTransformStrategy.equals(that.errorResponseTransformStrategy);
}
@Override
@@ -266,7 +291,9 @@ public class ServerConfig
inflateBufferSize,
compressionLevel,
enableForwardedRequestCustomizer,
- allowedHttpMethods
+ allowedHttpMethods,
+ errorResponseTransformStrategy,
+ showDetailedJettyErrors
);
}
@@ -288,7 +315,9 @@ public class ServerConfig
", inflateBufferSize=" + inflateBufferSize +
", compressionLevel=" + compressionLevel +
", enableForwardedRequestCustomizer=" +
enableForwardedRequestCustomizer +
- ", allowedMethods=" + allowedHttpMethods +
+ ", allowedHttpMethods=" + allowedHttpMethods +
+ ", errorResponseTransformStrategy=" +
errorResponseTransformStrategy +
+ ", showDetailedJettyErrors=" + showDetailedJettyErrors +
'}';
}
diff --git
a/server/src/main/java/org/apache/druid/server/initialization/jetty/CliIndexerServerModule.java
b/server/src/main/java/org/apache/druid/server/initialization/jetty/CliIndexerServerModule.java
index d8f6bd1..06561b6 100644
---
a/server/src/main/java/org/apache/druid/server/initialization/jetty/CliIndexerServerModule.java
+++
b/server/src/main/java/org/apache/druid/server/initialization/jetty/CliIndexerServerModule.java
@@ -160,7 +160,9 @@ public class CliIndexerServerModule implements Module
oldConfig.getInflateBufferSize(),
oldConfig.getCompressionLevel(),
oldConfig.isEnableForwardedRequestCustomizer(),
- oldConfig.getAllowedHttpMethods()
+ oldConfig.getAllowedHttpMethods(),
+ oldConfig.isShowDetailedJettyErrors(),
+ oldConfig.getErrorResponseTransformStrategy()
);
}
}
diff --git
a/server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java
b/server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java
index 351a4c0..41a00a2 100644
---
a/server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java
+++
b/server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java
@@ -68,10 +68,12 @@ import org.eclipse.jetty.server.ForwardedRequestCustomizer;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
+import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.SecureRequestCustomizer;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
+import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
@@ -82,6 +84,11 @@ import javax.net.ssl.SSLEngine;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
import javax.net.ssl.X509ExtendedTrustManager;
+import javax.servlet.RequestDispatcher;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
import java.security.KeyStore;
import java.security.cert.CRL;
import java.util.ArrayList;
@@ -466,6 +473,28 @@ public class JettyServerModule extends JerseyServletModule
Lifecycle.Stage.SERVER
);
+ if (!config.isShowDetailedJettyErrors()) {
+ server.setErrorHandler(new ErrorHandler() {
+ @Override
+ public boolean isShowServlet()
+ {
+ return false;
+ }
+
+ @Override
+ public void handle(
+ String target,
+ Request baseRequest,
+ HttpServletRequest request,
+ HttpServletResponse response
+ ) throws IOException, ServletException
+ {
+ request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, null);
+ super.handle(target, baseRequest, request, response);
+ }
+ });
+ }
+
return server;
}
diff --git
a/server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
b/server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
index 6cf326c..7de37d6 100644
---
a/server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
+++
b/server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
@@ -21,16 +21,22 @@ package org.apache.druid.server.security;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Strings;
+import org.apache.druid.common.exception.SanitizableException;
+
+import java.util.function.Function;
/**
* Throw this when a request is unauthorized and we want to send a 403
response back, Jersey exception mapper will
* take care of sending the response.
*/
-public class ForbiddenException extends RuntimeException
+public class ForbiddenException extends RuntimeException implements
SanitizableException
{
+ static final String DEFAULT_ERROR_MESSAGE = "Unauthorized.";
+
public ForbiddenException()
{
- super("Unauthorized.");
+ super(DEFAULT_ERROR_MESSAGE);
}
@JsonCreator
@@ -44,4 +50,15 @@ public class ForbiddenException extends RuntimeException
{
return super.getMessage();
}
+
+ @Override
+ public ForbiddenException sanitize(Function<String, String>
errorMessageTransformFunction)
+ {
+ String transformedErrorMessage =
errorMessageTransformFunction.apply(getMessage());
+ if (Strings.isNullOrEmpty(transformedErrorMessage)) {
+ return new ForbiddenException();
+ } else {
+ return new ForbiddenException(transformedErrorMessage);
+ }
+ }
}
diff --git
a/server/src/test/java/org/apache/druid/initialization/ServerConfigTest.java
b/server/src/test/java/org/apache/druid/initialization/ServerConfigTest.java
index 4c8c908..e542ca8 100644
--- a/server/src/test/java/org/apache/druid/initialization/ServerConfigTest.java
+++ b/server/src/test/java/org/apache/druid/initialization/ServerConfigTest.java
@@ -22,6 +22,7 @@ package org.apache.druid.initialization;
import com.google.common.collect.ImmutableList;
import nl.jqno.equalsverifier.EqualsVerifier;
import nl.jqno.equalsverifier.Warning;
+import
org.apache.druid.common.exception.AllowedRegexErrorResponseTransformStrategy;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.server.initialization.ServerConfig;
import org.junit.Assert;
@@ -57,13 +58,16 @@ public class ServerConfigTest
defaultConfig.getInflateBufferSize(),
defaultConfig.getCompressionLevel(),
true,
- ImmutableList.of(HttpMethod.OPTIONS)
+ ImmutableList.of(HttpMethod.OPTIONS),
+ true,
+ new AllowedRegexErrorResponseTransformStrategy(ImmutableList.of(".*"))
);
String modifiedConfigJson =
OBJECT_MAPPER.writeValueAsString(modifiedConfig);
ServerConfig modifiedConfig2 = OBJECT_MAPPER.readValue(modifiedConfigJson,
ServerConfig.class);
Assert.assertEquals(modifiedConfig, modifiedConfig2);
Assert.assertEquals(999, modifiedConfig2.getNumThreads());
Assert.assertEquals(888, modifiedConfig2.getQueueSize());
+ Assert.assertTrue(modifiedConfig2.getErrorResponseTransformStrategy()
instanceof AllowedRegexErrorResponseTransformStrategy);
Assert.assertTrue(modifiedConfig2.isEnableForwardedRequestCustomizer());
Assert.assertEquals(1, modifiedConfig2.getAllowedHttpMethods().size());
Assert.assertTrue(modifiedConfig2.getAllowedHttpMethods().contains(HttpMethod.OPTIONS));
diff --git
a/server/src/test/java/org/apache/druid/server/initialization/BaseJettyTest.java
b/server/src/test/java/org/apache/druid/server/initialization/BaseJettyTest.java
index 6e555a7..caf0900 100644
---
a/server/src/test/java/org/apache/druid/server/initialization/BaseJettyTest.java
+++
b/server/src/test/java/org/apache/druid/server/initialization/BaseJettyTest.java
@@ -76,7 +76,7 @@ public abstract class BaseJettyTest
protected int port = -1;
protected int tlsPort = -1;
- public static void setProperties()
+ protected void setProperties()
{
System.setProperty("druid.server.http.numThreads", "20");
System.setProperty("druid.server.http.maxIdleTime", "PT1S");
diff --git
a/server/src/test/java/org/apache/druid/server/initialization/JettyTest.java
b/server/src/test/java/org/apache/druid/server/initialization/JettyTest.java
index 4f2e90f..59c84b9 100644
--- a/server/src/test/java/org/apache/druid/server/initialization/JettyTest.java
+++ b/server/src/test/java/org/apache/druid/server/initialization/JettyTest.java
@@ -554,6 +554,13 @@ public class JettyTest extends BaseJettyTest
Assert.assertTrue(endpointIdentificationAlgorithm == null ||
endpointIdentificationAlgorithm.isEmpty());
}
+ @Test
+ public void testJettyErrorHandlerWithFilter()
+ {
+ // Response filter is disabled by default hence we show servlet information
+ Assert.assertTrue(server.getErrorHandler().isShowServlet());
+ }
+
private void waitForJettyServerModuleActiveConnectionsZero(JettyServerModule
jsm) throws InterruptedException
{
// it can take a bit to close the connection, so maybe sleep for a while
and hope it closes
diff --git
a/server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
b/server/src/test/java/org/apache/druid/server/initialization/JettyWithResponseFilterEnabledTest.java
similarity index 57%
copy from
server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
copy to
server/src/test/java/org/apache/druid/server/initialization/JettyWithResponseFilterEnabledTest.java
index 6cf326c..b788e71 100644
---
a/server/src/main/java/org/apache/druid/server/security/ForbiddenException.java
+++
b/server/src/test/java/org/apache/druid/server/initialization/JettyWithResponseFilterEnabledTest.java
@@ -17,31 +17,25 @@
* under the License.
*/
-package org.apache.druid.server.security;
+package org.apache.druid.server.initialization;
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonProperty;
+import org.junit.Assert;
+import org.junit.Test;
-/**
- * Throw this when a request is unauthorized and we want to send a 403
response back, Jersey exception mapper will
- * take care of sending the response.
- */
-public class ForbiddenException extends RuntimeException
+public class JettyWithResponseFilterEnabledTest extends JettyTest
{
- public ForbiddenException()
- {
- super("Unauthorized.");
- }
-
- @JsonCreator
- public ForbiddenException(@JsonProperty("errorMessage") String msg)
+ @Override
+ public void setProperties()
{
- super(msg);
+ System.setProperty("druid.server.http.showDetailedJettyErrors", "false");
+ super.setProperties();
}
- @JsonProperty
- public String getErrorMessage()
+ @Test
+ @Override
+ public void testJettyErrorHandlerWithFilter()
{
- return super.getMessage();
+ // Response filter is enabled by config hence we do not show servlet
information
+ Assert.assertFalse(server.getErrorHandler().isShowServlet());
}
}
diff --git
a/server/src/test/java/org/apache/druid/server/security/ForbiddenExceptionTest.java
b/server/src/test/java/org/apache/druid/server/security/ForbiddenExceptionTest.java
new file mode 100644
index 0000000..01179e6
--- /dev/null
+++
b/server/src/test/java/org/apache/druid/server/security/ForbiddenExceptionTest.java
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.security;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.function.Function;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ForbiddenExceptionTest
+{
+ private static final String ERROR_MESSAGE_ORIGINAL = "aaaa";
+ private static final String ERROR_MESSAGE_TRANSFORMED = "bbbb";
+
+ @Mock
+ private Function<String, String> trasformFunction;
+
+ @Test
+ public void testSanitizeWithTransformFunctionReturningNull()
+ {
+
Mockito.when(trasformFunction.apply(ArgumentMatchers.eq(ERROR_MESSAGE_ORIGINAL))).thenReturn(null);
+ ForbiddenException forbiddenException = new
ForbiddenException(ERROR_MESSAGE_ORIGINAL);
+ ForbiddenException actual = forbiddenException.sanitize(trasformFunction);
+ Assert.assertNotNull(actual);
+ Assert.assertEquals(actual.getMessage(),
ForbiddenException.DEFAULT_ERROR_MESSAGE);
+
Mockito.verify(trasformFunction).apply(ArgumentMatchers.eq(ERROR_MESSAGE_ORIGINAL));
+ Mockito.verifyNoMoreInteractions(trasformFunction);
+ }
+
+ @Test
+ public void testSanitizeWithTransformFunctionReturningNewString()
+ {
+
Mockito.when(trasformFunction.apply(ArgumentMatchers.eq(ERROR_MESSAGE_ORIGINAL))).thenReturn(ERROR_MESSAGE_TRANSFORMED);
+ ForbiddenException forbiddenException = new
ForbiddenException(ERROR_MESSAGE_ORIGINAL);
+ ForbiddenException actual = forbiddenException.sanitize(trasformFunction);
+ Assert.assertNotNull(actual);
+ Assert.assertEquals(actual.getMessage(), ERROR_MESSAGE_TRANSFORMED);
+
Mockito.verify(trasformFunction).apply(ArgumentMatchers.eq(ERROR_MESSAGE_ORIGINAL));
+ Mockito.verifyNoMoreInteractions(trasformFunction);
+ }
+}
diff --git
a/services/src/main/java/org/apache/druid/server/AsyncQueryForwardingServlet.java
b/services/src/main/java/org/apache/druid/server/AsyncQueryForwardingServlet.java
index 8365ba1..87c1c15 100644
---
a/services/src/main/java/org/apache/druid/server/AsyncQueryForwardingServlet.java
+++
b/services/src/main/java/org/apache/druid/server/AsyncQueryForwardingServlet.java
@@ -42,8 +42,10 @@ import
org.apache.druid.java.util.emitter.service.ServiceEmitter;
import org.apache.druid.query.DruidMetrics;
import org.apache.druid.query.GenericQueryMetricsFactory;
import org.apache.druid.query.Query;
+import org.apache.druid.query.QueryInterruptedException;
import org.apache.druid.query.QueryMetrics;
import org.apache.druid.query.QueryToolChestWarehouse;
+import org.apache.druid.server.initialization.ServerConfig;
import org.apache.druid.server.log.RequestLogger;
import org.apache.druid.server.metrics.QueryCountStatsProvider;
import org.apache.druid.server.router.QueryHostFinder;
@@ -102,17 +104,18 @@ public class AsyncQueryForwardingServlet extends
AsyncProxyServlet implements Qu
private final AtomicLong failedQueryCount = new AtomicLong();
private final AtomicLong interruptedQueryCount = new AtomicLong();
- private static void handleException(HttpServletResponse response,
ObjectMapper objectMapper, Exception exception)
+ @VisibleForTesting
+ void handleException(HttpServletResponse response, ObjectMapper
objectMapper, Exception exception)
throws IOException
{
+ QueryInterruptedException exceptionToReport =
QueryInterruptedException.wrapIfNeeded(exception);
+ LOG.warn(exceptionToReport, "Unexpected exception occurs");
if (!response.isCommitted()) {
- final String errorMessage = exception.getMessage() == null ? "null
exception" : exception.getMessage();
-
response.resetBuffer();
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
objectMapper.writeValue(
response.getOutputStream(),
- ImmutableMap.of("error", errorMessage)
+
serverConfig.getErrorResponseTransformStrategy().transformIfNeeded(exceptionToReport)
);
}
response.flushBuffer();
@@ -129,6 +132,7 @@ public class AsyncQueryForwardingServlet extends
AsyncProxyServlet implements Qu
private final GenericQueryMetricsFactory queryMetricsFactory;
private final AuthenticatorMapper authenticatorMapper;
private final ProtobufTranslation protobufTranslation;
+ private final ServerConfig serverConfig;
private final boolean routeSqlByStrategy;
@@ -146,7 +150,8 @@ public class AsyncQueryForwardingServlet extends
AsyncProxyServlet implements Qu
RequestLogger requestLogger,
GenericQueryMetricsFactory queryMetricsFactory,
AuthenticatorMapper authenticatorMapper,
- Properties properties
+ Properties properties,
+ final ServerConfig serverConfig
)
{
this.warehouse = warehouse;
@@ -163,6 +168,7 @@ public class AsyncQueryForwardingServlet extends
AsyncProxyServlet implements Qu
this.routeSqlByStrategy = Boolean.parseBoolean(
properties.getProperty(PROPERTY_SQL_ENABLE,
PROPERTY_SQL_ENABLE_DEFAULT)
);
+ this.serverConfig = serverConfig;
}
@Override
@@ -320,7 +326,8 @@ public class AsyncQueryForwardingServlet extends
AsyncProxyServlet implements Qu
interruptedQueryCount.incrementAndGet();
}
- private void handleQueryParseException(
+ @VisibleForTesting
+ void handleQueryParseException(
HttpServletRequest request,
HttpServletResponse response,
ObjectMapper objectMapper,
@@ -328,11 +335,12 @@ public class AsyncQueryForwardingServlet extends
AsyncProxyServlet implements Qu
boolean isNativeQuery
) throws IOException
{
- LOG.warn(parseException, "Exception parsing query");
+ QueryInterruptedException exceptionToReport =
QueryInterruptedException.wrapIfNeeded(parseException);
+ LOG.warn(exceptionToReport, "Exception parsing query");
// Log the error message
- final String errorMessage = parseException.getMessage() == null
- ? "no error message" :
parseException.getMessage();
+ final String errorMessage = exceptionToReport.getMessage() == null
+ ? "no error message" :
exceptionToReport.getMessage();
if (isNativeQuery) {
requestLogger.logNativeQuery(
RequestLogLine.forNative(
@@ -359,7 +367,7 @@ public class AsyncQueryForwardingServlet extends
AsyncProxyServlet implements Qu
response.setContentType(MediaType.APPLICATION_JSON);
objectMapper.writeValue(
response.getOutputStream(),
- ImmutableMap.of("error", errorMessage)
+
serverConfig.getErrorResponseTransformStrategy().transformIfNeeded(exceptionToReport)
);
}
diff --git
a/services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java
b/services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java
index 9a92289..634f8a4 100644
---
a/services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java
+++
b/services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java
@@ -32,6 +32,8 @@ import com.google.inject.Module;
import com.google.inject.servlet.GuiceFilter;
import org.apache.calcite.avatica.Meta;
import org.apache.calcite.avatica.remote.Service;
+import
org.apache.druid.common.exception.AllowedRegexErrorResponseTransformStrategy;
+import org.apache.druid.common.exception.ErrorResponseTransformStrategy;
import org.apache.druid.common.utils.SocketUtil;
import org.apache.druid.guice.GuiceInjectors;
import org.apache.druid.guice.Jerseys;
@@ -50,6 +52,8 @@ import
org.apache.druid.query.DefaultGenericQueryMetricsFactory;
import org.apache.druid.query.Druids;
import org.apache.druid.query.MapQueryToolChestWarehouse;
import org.apache.druid.query.Query;
+import org.apache.druid.query.QueryException;
+import org.apache.druid.query.QueryInterruptedException;
import org.apache.druid.query.timeseries.TimeseriesQuery;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.server.initialization.BaseJettyTest;
@@ -78,13 +82,18 @@ import org.eclipse.jetty.servlet.ServletHolder;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mockito;
import javax.servlet.ReadListener;
import javax.servlet.ServletInputStream;
+import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.ByteArrayInputStream;
+import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URL;
@@ -224,6 +233,253 @@ public class AsyncQueryForwardingServletTest extends
BaseJettyTest
verifyServletCallsForQuery(query, false, hostFinder, new Properties());
}
+ @Test
+ public void testHandleExceptionWithFilterDisabled() throws Exception
+ {
+ String errorMessage = "test exception message";
+ ObjectMapper mockMapper = Mockito.mock(ObjectMapper.class);
+ HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+ ServletOutputStream outputStream = Mockito.mock(ServletOutputStream.class);
+ Mockito.when(response.getOutputStream()).thenReturn(outputStream);
+ final AsyncQueryForwardingServlet servlet = new
AsyncQueryForwardingServlet(
+ new MapQueryToolChestWarehouse(ImmutableMap.of()),
+ mockMapper,
+ TestHelper.makeSmileMapper(),
+ null,
+ null,
+ null,
+ new NoopServiceEmitter(),
+ new NoopRequestLogger(),
+ new DefaultGenericQueryMetricsFactory(),
+ new AuthenticatorMapper(ImmutableMap.of()),
+ new Properties(),
+ new ServerConfig()
+ );
+ Exception testException = new IllegalStateException(errorMessage);
+ servlet.handleException(response, mockMapper, testException);
+ ArgumentCaptor<Exception> captor =
ArgumentCaptor.forClass(Exception.class);
+ Mockito.verify(mockMapper).writeValue(ArgumentMatchers.eq(outputStream),
captor.capture());
+ Assert.assertTrue(captor.getValue() instanceof QueryException);
+ Assert.assertEquals(QueryInterruptedException.UNKNOWN_EXCEPTION,
((QueryException) captor.getValue()).getErrorCode());
+ Assert.assertEquals(errorMessage, captor.getValue().getMessage());
+ Assert.assertEquals(IllegalStateException.class.getName(),
((QueryException) captor.getValue()).getErrorClass());
+ }
+
+ @Test
+ public void testHandleExceptionWithFilterEnabled() throws Exception
+ {
+ String errorMessage = "test exception message";
+ ObjectMapper mockMapper = Mockito.mock(ObjectMapper.class);
+ HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+ ServletOutputStream outputStream = Mockito.mock(ServletOutputStream.class);
+ Mockito.when(response.getOutputStream()).thenReturn(outputStream);
+ final AsyncQueryForwardingServlet servlet = new
AsyncQueryForwardingServlet(
+ new MapQueryToolChestWarehouse(ImmutableMap.of()),
+ mockMapper,
+ TestHelper.makeSmileMapper(),
+ null,
+ null,
+ null,
+ new NoopServiceEmitter(),
+ new NoopRequestLogger(),
+ new DefaultGenericQueryMetricsFactory(),
+ new AuthenticatorMapper(ImmutableMap.of()),
+ new Properties(),
+ new ServerConfig() {
+ @Override
+ public boolean isShowDetailedJettyErrors()
+ {
+ return true;
+ }
+
+ @Override
+ public ErrorResponseTransformStrategy
getErrorResponseTransformStrategy()
+ {
+ return new
AllowedRegexErrorResponseTransformStrategy(ImmutableList.of());
+ }
+ }
+ );
+ Exception testException = new IllegalStateException(errorMessage);
+ servlet.handleException(response, mockMapper, testException);
+ ArgumentCaptor<Exception> captor =
ArgumentCaptor.forClass(Exception.class);
+ Mockito.verify(mockMapper).writeValue(ArgumentMatchers.eq(outputStream),
captor.capture());
+ Assert.assertTrue(captor.getValue() instanceof QueryException);
+ Assert.assertEquals(QueryInterruptedException.UNKNOWN_EXCEPTION,
((QueryException) captor.getValue()).getErrorCode());
+ Assert.assertNull(captor.getValue().getMessage());
+ Assert.assertNull(((QueryException) captor.getValue()).getErrorClass());
+ Assert.assertNull(((QueryException) captor.getValue()).getHost());
+ }
+
+ @Test
+ public void
testHandleExceptionWithFilterEnabledButMessageMatchAllowedRegex() throws
Exception
+ {
+ String errorMessage = "test exception message";
+ ObjectMapper mockMapper = Mockito.mock(ObjectMapper.class);
+ HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+ ServletOutputStream outputStream = Mockito.mock(ServletOutputStream.class);
+ Mockito.when(response.getOutputStream()).thenReturn(outputStream);
+ final AsyncQueryForwardingServlet servlet = new
AsyncQueryForwardingServlet(
+ new MapQueryToolChestWarehouse(ImmutableMap.of()),
+ mockMapper,
+ TestHelper.makeSmileMapper(),
+ null,
+ null,
+ null,
+ new NoopServiceEmitter(),
+ new NoopRequestLogger(),
+ new DefaultGenericQueryMetricsFactory(),
+ new AuthenticatorMapper(ImmutableMap.of()),
+ new Properties(),
+ new ServerConfig() {
+ @Override
+ public boolean isShowDetailedJettyErrors()
+ {
+ return true;
+ }
+
+ @Override
+ public ErrorResponseTransformStrategy
getErrorResponseTransformStrategy()
+ {
+ return new
AllowedRegexErrorResponseTransformStrategy(ImmutableList.of("test .*"));
+ }
+ }
+ );
+ Exception testException = new IllegalStateException(errorMessage);
+ servlet.handleException(response, mockMapper, testException);
+ ArgumentCaptor<Exception> captor =
ArgumentCaptor.forClass(Exception.class);
+ Mockito.verify(mockMapper).writeValue(ArgumentMatchers.eq(outputStream),
captor.capture());
+ Assert.assertTrue(captor.getValue() instanceof QueryException);
+ Assert.assertEquals(QueryInterruptedException.UNKNOWN_EXCEPTION,
((QueryException) captor.getValue()).getErrorCode());
+ Assert.assertEquals(errorMessage, captor.getValue().getMessage());
+ Assert.assertNull(((QueryException) captor.getValue()).getErrorClass());
+ Assert.assertNull(((QueryException) captor.getValue()).getHost());
+ }
+
+ @Test
+ public void testHandleQueryParseExceptionWithFilterDisabled() throws
Exception
+ {
+ String errorMessage = "test exception message";
+ ObjectMapper mockMapper = Mockito.mock(ObjectMapper.class);
+ HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+ ServletOutputStream outputStream = Mockito.mock(ServletOutputStream.class);
+ Mockito.when(response.getOutputStream()).thenReturn(outputStream);
+ final AsyncQueryForwardingServlet servlet = new
AsyncQueryForwardingServlet(
+ new MapQueryToolChestWarehouse(ImmutableMap.of()),
+ mockMapper,
+ TestHelper.makeSmileMapper(),
+ null,
+ null,
+ null,
+ new NoopServiceEmitter(),
+ new NoopRequestLogger(),
+ new DefaultGenericQueryMetricsFactory(),
+ new AuthenticatorMapper(ImmutableMap.of()),
+ new Properties(),
+ new ServerConfig()
+ );
+ IOException testException = new IOException(errorMessage);
+ servlet.handleQueryParseException(request, response, mockMapper,
testException, false);
+ ArgumentCaptor<Exception> captor =
ArgumentCaptor.forClass(Exception.class);
+ Mockito.verify(mockMapper).writeValue(ArgumentMatchers.eq(outputStream),
captor.capture());
+ Assert.assertTrue(captor.getValue() instanceof QueryException);
+ Assert.assertEquals(QueryInterruptedException.UNKNOWN_EXCEPTION,
((QueryException) captor.getValue()).getErrorCode());
+ Assert.assertEquals(errorMessage, captor.getValue().getMessage());
+ Assert.assertEquals(IOException.class.getName(), ((QueryException)
captor.getValue()).getErrorClass());
+ }
+
+ @Test
+ public void testHandleQueryParseExceptionWithFilterEnabled() throws Exception
+ {
+ String errorMessage = "test exception message";
+ ObjectMapper mockMapper = Mockito.mock(ObjectMapper.class);
+ HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+ ServletOutputStream outputStream = Mockito.mock(ServletOutputStream.class);
+ Mockito.when(response.getOutputStream()).thenReturn(outputStream);
+ final AsyncQueryForwardingServlet servlet = new
AsyncQueryForwardingServlet(
+ new MapQueryToolChestWarehouse(ImmutableMap.of()),
+ mockMapper,
+ TestHelper.makeSmileMapper(),
+ null,
+ null,
+ null,
+ new NoopServiceEmitter(),
+ new NoopRequestLogger(),
+ new DefaultGenericQueryMetricsFactory(),
+ new AuthenticatorMapper(ImmutableMap.of()),
+ new Properties(),
+ new ServerConfig() {
+ @Override
+ public boolean isShowDetailedJettyErrors()
+ {
+ return true;
+ }
+
+ @Override
+ public ErrorResponseTransformStrategy
getErrorResponseTransformStrategy()
+ {
+ return new
AllowedRegexErrorResponseTransformStrategy(ImmutableList.of());
+ }
+ }
+ );
+ IOException testException = new IOException(errorMessage);
+ servlet.handleQueryParseException(request, response, mockMapper,
testException, false);
+ ArgumentCaptor<Exception> captor =
ArgumentCaptor.forClass(Exception.class);
+ Mockito.verify(mockMapper).writeValue(ArgumentMatchers.eq(outputStream),
captor.capture());
+ Assert.assertTrue(captor.getValue() instanceof QueryException);
+ Assert.assertEquals(QueryInterruptedException.UNKNOWN_EXCEPTION,
((QueryException) captor.getValue()).getErrorCode());
+ Assert.assertNull(captor.getValue().getMessage());
+ Assert.assertNull(((QueryException) captor.getValue()).getErrorClass());
+ Assert.assertNull(((QueryException) captor.getValue()).getHost());
+ }
+
+ @Test
+ public void
testHandleQueryParseExceptionWithFilterEnabledButMessageMatchAllowedRegex()
throws Exception
+ {
+ String errorMessage = "test exception message";
+ ObjectMapper mockMapper = Mockito.mock(ObjectMapper.class);
+ HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+ ServletOutputStream outputStream = Mockito.mock(ServletOutputStream.class);
+ Mockito.when(response.getOutputStream()).thenReturn(outputStream);
+ final AsyncQueryForwardingServlet servlet = new
AsyncQueryForwardingServlet(
+ new MapQueryToolChestWarehouse(ImmutableMap.of()),
+ mockMapper,
+ TestHelper.makeSmileMapper(),
+ null,
+ null,
+ null,
+ new NoopServiceEmitter(),
+ new NoopRequestLogger(),
+ new DefaultGenericQueryMetricsFactory(),
+ new AuthenticatorMapper(ImmutableMap.of()),
+ new Properties(),
+ new ServerConfig() {
+ @Override
+ public boolean isShowDetailedJettyErrors()
+ {
+ return true;
+ }
+
+ @Override
+ public ErrorResponseTransformStrategy
getErrorResponseTransformStrategy()
+ {
+ return new
AllowedRegexErrorResponseTransformStrategy(ImmutableList.of("test .*"));
+ }
+ }
+ );
+ IOException testException = new IOException(errorMessage);
+ servlet.handleQueryParseException(request, response, mockMapper,
testException, false);
+ ArgumentCaptor<Exception> captor =
ArgumentCaptor.forClass(Exception.class);
+ Mockito.verify(mockMapper).writeValue(ArgumentMatchers.eq(outputStream),
captor.capture());
+ Assert.assertTrue(captor.getValue() instanceof QueryException);
+ Assert.assertEquals(QueryInterruptedException.UNKNOWN_EXCEPTION,
((QueryException) captor.getValue()).getErrorCode());
+ Assert.assertEquals(errorMessage, captor.getValue().getMessage());
+ Assert.assertNull(((QueryException) captor.getValue()).getErrorClass());
+ Assert.assertNull(((QueryException) captor.getValue()).getHost());
+ }
+
/**
* Verifies that the Servlet calls the right methods the right number of
times.
*/
@@ -296,7 +552,8 @@ public class AsyncQueryForwardingServletTest extends
BaseJettyTest
new NoopRequestLogger(),
new DefaultGenericQueryMetricsFactory(),
new AuthenticatorMapper(ImmutableMap.of()),
- properties
+ properties,
+ new ServerConfig()
)
{
@Override
@@ -389,7 +646,8 @@ public class AsyncQueryForwardingServletTest extends
BaseJettyTest
new NoopRequestLogger(),
new DefaultGenericQueryMetricsFactory(),
new AuthenticatorMapper(ImmutableMap.of()),
- new Properties()
+ new Properties(),
+ new ServerConfig()
)
{
@Override
diff --git a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
index 1d20f5d..2a070f4 100644
--- a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
+++ b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
@@ -26,6 +26,7 @@ import com.google.common.collect.Iterables;
import com.google.common.io.CountingOutputStream;
import com.google.inject.Inject;
import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.druid.common.exception.SanitizableException;
import org.apache.druid.guice.annotations.Json;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.StringUtils;
@@ -39,6 +40,7 @@ import org.apache.druid.query.QueryInterruptedException;
import org.apache.druid.query.QueryTimeoutException;
import org.apache.druid.query.QueryUnsupportedException;
import org.apache.druid.query.ResourceLimitExceededException;
+import org.apache.druid.server.initialization.ServerConfig;
import org.apache.druid.server.security.Access;
import org.apache.druid.server.security.AuthorizationUtils;
import org.apache.druid.server.security.AuthorizerMapper;
@@ -77,19 +79,22 @@ public class SqlResource
private final AuthorizerMapper authorizerMapper;
private final SqlLifecycleFactory sqlLifecycleFactory;
private final SqlLifecycleManager sqlLifecycleManager;
+ private final ServerConfig serverConfig;
@Inject
public SqlResource(
@Json ObjectMapper jsonMapper,
AuthorizerMapper authorizerMapper,
SqlLifecycleFactory sqlLifecycleFactory,
- SqlLifecycleManager sqlLifecycleManager
+ SqlLifecycleManager sqlLifecycleManager,
+ ServerConfig serverConfig
)
{
this.jsonMapper = Preconditions.checkNotNull(jsonMapper, "jsonMapper");
this.authorizerMapper = Preconditions.checkNotNull(authorizerMapper,
"authorizerMapper");
this.sqlLifecycleFactory = Preconditions.checkNotNull(sqlLifecycleFactory,
"sqlLifecycleFactory");
this.sqlLifecycleManager = Preconditions.checkNotNull(sqlLifecycleManager,
"sqlLifecycleManager");
+ this.serverConfig = serverConfig;
}
@POST
@@ -186,7 +191,7 @@ public class SqlResource
}
catch (ForbiddenException e) {
endLifecycleWithoutEmittingMetrics(sqlQueryId, lifecycle);
- throw e; // let ForbiddenExceptionMapper handle this
+ throw (ForbiddenException)
serverConfig.getErrorResponseTransformStrategy().transformIfNeeded(e); // let
ForbiddenExceptionMapper handle this
}
catch (Exception e) {
log.warn(e, "Failed to handle query: %s", sqlQuery);
@@ -230,11 +235,15 @@ public class SqlResource
sqlLifecycleManager.remove(sqlQueryId, lifecycle);
}
- private Response buildNonOkResponse(int status, Exception e) throws
JsonProcessingException
+ private Response buildNonOkResponse(int status, SanitizableException e)
throws JsonProcessingException
{
return Response.status(status)
.type(MediaType.APPLICATION_JSON_TYPE)
- .entity(jsonMapper.writeValueAsBytes(e))
+ .entity(
+ jsonMapper.writeValueAsBytes(
+
serverConfig.getErrorResponseTransformStrategy().transformIfNeeded(e)
+ )
+ )
.build();
}
diff --git a/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java
b/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java
index c7387ac..f52335d 100644
--- a/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java
@@ -30,6 +30,8 @@ import com.google.common.util.concurrent.MoreExecutors;
import org.apache.calcite.avatica.SqlType;
import org.apache.calcite.tools.RelConversionException;
import org.apache.druid.common.config.NullHandling;
+import
org.apache.druid.common.exception.AllowedRegexErrorResponseTransformStrategy;
+import org.apache.druid.common.exception.ErrorResponseTransformStrategy;
import org.apache.druid.common.guava.SettableSupplier;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.ISE;
@@ -121,6 +123,7 @@ public class SqlResourceTest extends CalciteTestBase
private HttpServletRequest req;
private ListeningExecutorService executorService;
private SqlLifecycleManager lifecycleManager;
+ private SqlLifecycleFactory sqlLifecycleFactory;
private CountDownLatch lifecycleAddLatch;
private final SettableSupplier<NonnullPair<CountDownLatch, Boolean>>
validateAndAuthorizeLatchSupplier = new SettableSupplier<>();
@@ -236,34 +239,36 @@ public class SqlResourceTest extends CalciteTestBase
}
};
final ServiceEmitter emitter = new NoopServiceEmitter();
- resource = new SqlResource(
- JSON_MAPPER,
- CalciteTests.TEST_AUTHORIZER_MAPPER,
- new SqlLifecycleFactory(
+ sqlLifecycleFactory = new SqlLifecycleFactory(
+ plannerFactory,
+ emitter,
+ testRequestLogger,
+ scheduler
+ )
+ {
+ @Override
+ public SqlLifecycle factorize()
+ {
+ return new TestSqlLifecycle(
plannerFactory,
emitter,
testRequestLogger,
- scheduler
- )
- {
- @Override
- public SqlLifecycle factorize()
- {
- return new TestSqlLifecycle(
- plannerFactory,
- emitter,
- testRequestLogger,
- scheduler,
- System.currentTimeMillis(),
- System.nanoTime(),
- validateAndAuthorizeLatchSupplier,
- planLatchSupplier,
- executeLatchSupplier,
- sequenceMapFnSupplier
- );
- }
- },
- lifecycleManager
+ scheduler,
+ System.currentTimeMillis(),
+ System.nanoTime(),
+ validateAndAuthorizeLatchSupplier,
+ planLatchSupplier,
+ executeLatchSupplier,
+ sequenceMapFnSupplier
+ );
+ }
+ };
+ resource = new SqlResource(
+ JSON_MAPPER,
+ CalciteTests.TEST_AUTHORIZER_MAPPER,
+ sqlLifecycleFactory,
+ lifecycleManager,
+ new ServerConfig()
);
}
@@ -969,6 +974,44 @@ public class SqlResourceTest extends CalciteTestBase
}
@Test
+ public void testUnsupportedQueryThrowsExceptionWithFilterResponse() throws
Exception
+ {
+ resource = new SqlResource(
+ JSON_MAPPER,
+ CalciteTests.TEST_AUTHORIZER_MAPPER,
+ sqlLifecycleFactory,
+ lifecycleManager,
+ new ServerConfig() {
+ @Override
+ public boolean isShowDetailedJettyErrors()
+ {
+ return true;
+ }
+ @Override
+ public ErrorResponseTransformStrategy
getErrorResponseTransformStrategy()
+ {
+ return new
AllowedRegexErrorResponseTransformStrategy(ImmutableList.of());
+ }
+ }
+ );
+
+ String errorMessage = "This will be support in Druid 9999";
+ SqlQuery badQuery = EasyMock.createMock(SqlQuery.class);
+ EasyMock.expect(badQuery.getQuery()).andReturn("SELECT ANSWER TO LIFE");
+
EasyMock.expect(badQuery.getContext()).andReturn(ImmutableMap.of("sqlQueryId",
"id"));
+ EasyMock.expect(badQuery.getParameterList()).andThrow(new
QueryUnsupportedException(errorMessage));
+ EasyMock.replay(badQuery);
+ final QueryException exception = doPost(badQuery).lhs;
+
+ Assert.assertNotNull(exception);
+ Assert.assertNull(exception.getMessage());
+ Assert.assertNull(exception.getHost());
+ Assert.assertEquals(exception.getErrorCode(),
QueryUnsupportedException.ERROR_CODE);
+ Assert.assertNull(exception.getErrorClass());
+ Assert.assertTrue(lifecycleManager.getAll("id").isEmpty());
+ }
+
+ @Test
public void testTooManyRequests() throws Exception
{
sleep = true;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
