You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by za...@apache.org on 2021/07/23 15:11:26 UTC
[calcite-avatica] branch master updated: [CALCITE-4646] Bump Jetty
version to 9.4.42.v20210604 (Istvan Toth)
This is an automated email from the ASF dual-hosted git repository.
zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git
The following commit(s) were added to refs/heads/master by this push:
new 945d06c [CALCITE-4646] Bump Jetty version to 9.4.42.v20210604 (Istvan Toth)
945d06c is described below
commit 945d06c2a721327445f597ccdb302189e35feb8e
Author: Istvan Toth <st...@apache.org>
AuthorDate: Fri Jun 11 09:45:29 2021 +0200
[CALCITE-4646] Bump Jetty version to 9.4.42.v20210604 (Istvan Toth)
Updating jetty.version causes several SPENGO tests to fail
due to a breaking change in Jetty. The threads created by
Jetty no longer run as the Subject Jetty was started as.
1. Add new custom ThreadFactrory to obtain the old
behavior and overcome failures.
2. Copy most of org.apache.calcite.util.Sources to Avatica,
and use them in tests.
3. Remove illegal leading slash in the file paths to avoid
test failures on Windows.
Close apache/calcite-avatica#147
---
.../org/apache/calcite/avatica/util/Source.java | 72 ++++++
.../org/apache/calcite/avatica/util/Sources.java | 274 +++++++++++++++++++++
gradle.properties | 2 +-
.../apache/calcite/avatica/server/HttpServer.java | 7 +-
.../SubjectPreservingPrivilegedThreadFactory.java | 60 +++++
.../avatica/server/BasicAuthHttpServerTest.java | 7 +-
.../avatica/server/DigestAuthHttpServerTest.java | 6 +-
.../calcite/avatica/server/HttpAuthBase.java | 16 +-
8 files changed, 424 insertions(+), 20 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/avatica/util/Source.java b/core/src/main/java/org/apache/calcite/avatica/util/Source.java
new file mode 100644
index 0000000..f7dcfe0
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/avatica/util/Source.java
@@ -0,0 +1,72 @@
+/*
+ * 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.calcite.avatica.util;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+import java.net.URL;
+
+/**
+ * Source of data.
+ */
+public interface Source {
+ URL url();
+ File file();
+ String path();
+ Reader reader() throws IOException;
+ InputStream openStream() throws IOException;
+ String protocol();
+
+ /** Looks for a suffix on a path and returns
+ * either the path with the suffix removed
+ * or the original path. */
+ Source trim(String suffix);
+
+ /** Looks for a suffix on a path and returns
+ * either the path with the suffix removed
+ * or null. */
+ Source trimOrNull(String suffix);
+
+ /** Returns a source whose path concatenates this with a child.
+ *
+ * <p>For example,
+ * <ul>
+ * <li>source("/foo").append(source("bar"))
+ * returns source("/foo/bar")
+ * <li>source("/foo").append(source("/bar"))
+ * returns source("/bar")
+ * because "/bar" was already absolute
+ * </ul>
+ */
+ Source append(Source child);
+
+ /** Returns a relative source, if this source is a child of a given base.
+ *
+ * <p>For example,
+ * <ul>
+ * <li>source("/foo/bar").relative(source("/foo"))
+ * returns source("bar")
+ * <li>source("/baz/bar").relative(source("/foo"))
+ * returns source("/baz/bar")
+ * </ul>
+ */
+ Source relative(Source source);
+}
+
+// End Source.java
diff --git a/core/src/main/java/org/apache/calcite/avatica/util/Sources.java b/core/src/main/java/org/apache/calcite/avatica/util/Sources.java
new file mode 100644
index 0000000..46762ea
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/avatica/util/Sources.java
@@ -0,0 +1,274 @@
+/*
+ * 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.calcite.avatica.util;
+
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
+import java.util.Objects;
+import java.util.zip.GZIPInputStream;
+
+/**
+ * Utilities for {@link Source}.
+ */
+public abstract class Sources {
+ private Sources() {}
+
+ public static Source of(File file) {
+ return new FileSource(file);
+ }
+
+ public static Source of(URL url) {
+ return new FileSource(url);
+ }
+
+
+ public static Source file(File baseDirectory, String fileName) {
+ final File file = new File(fileName);
+ if (baseDirectory != null && !file.isAbsolute()) {
+ return of(new File(baseDirectory, fileName));
+ } else {
+ return of(file);
+ }
+ }
+
+
+ public static Source url(String url) {
+ try {
+ return of(new URL(url));
+ } catch (MalformedURLException | IllegalArgumentException e) {
+ throw new RuntimeException("Malformed URL: '" + url + "'", e);
+ }
+ }
+
+ /** Looks for a suffix on a path and returns
+ * either the path with the suffix removed
+ * or null. */
+ private static String trimOrNull(String s, String suffix) {
+ return s.endsWith(suffix)
+ ? s.substring(0, s.length() - suffix.length())
+ : null;
+ }
+
+ private static boolean isFile(Source source) {
+ return source.protocol().equals("file");
+ }
+
+ /** Implementation of {@link Source} on the top of a {@link File} or
+ * {@link URL}. */
+ private static class FileSource implements Source {
+ private final File file;
+ private final URL url;
+
+ /**
+ * A flag indicating if the url is deduced from the file object.
+ */
+ private final boolean urlGenerated;
+
+ private FileSource(URL url) {
+ this.url = Objects.requireNonNull(url, "url");
+ this.file = urlToFile(url);
+ this.urlGenerated = false;
+ }
+
+ private FileSource(File file) {
+ this.file = Objects.requireNonNull(file, "file");
+ this.url = fileToUrl(file);
+ this.urlGenerated = true;
+ }
+
+ private File fileNonNull() {
+ return Objects.requireNonNull(file, "file");
+ }
+
+ private static File urlToFile(URL url) {
+ if (!"file".equals(url.getProtocol())) {
+ return null;
+ }
+ URI uri;
+ try {
+ uri = url.toURI();
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException("Unable to convert URL " + url + " to URI", e);
+ }
+ if (uri.isOpaque()) {
+ // It is like file:test%20file.c++
+ // getSchemeSpecificPart would return "test file.c++"
+ return new File(uri.getSchemeSpecificPart());
+ }
+ // See https://stackoverflow.com/a/17870390/1261287
+ return Paths.get(uri).toFile();
+ }
+
+ private static URL fileToUrl(File file) {
+ String filePath = file.getPath();
+ if (!file.isAbsolute()) {
+ // convert relative file paths
+ filePath = filePath.replace(File.separatorChar, '/');
+ if (file.isDirectory() && !filePath.endsWith("/")) {
+ filePath += "/";
+ }
+ try {
+ // We need to encode path. For instance, " " should become "%20"
+ // That is why java.net.URLEncoder.encode(java.lang.String, java.lang.String) is not
+ // suitable because it replaces " " with "+".
+ String encodedPath = new URI(null, null, filePath, null).getRawPath();
+ return new URL("file", null, 0, encodedPath);
+ } catch (MalformedURLException | URISyntaxException e) {
+ throw new IllegalArgumentException("Unable to create URL for file " + filePath, e);
+ }
+ }
+
+ URI uri = null;
+ try {
+ // convert absolute file paths
+ uri = file.toURI();
+ return uri.toURL();
+ } catch (SecurityException e) {
+ throw new IllegalArgumentException("No access to the underlying file " + filePath, e);
+ } catch (MalformedURLException e) {
+ throw new IllegalArgumentException("Unable to convert URI " + uri + " to URL", e);
+ }
+ }
+
+ @Override public String toString() {
+ return (urlGenerated ? fileNonNull() : url).toString();
+ }
+
+ @Override public URL url() {
+ return url;
+ }
+
+ @Override public File file() {
+ if (file == null) {
+ throw new UnsupportedOperationException();
+ }
+ return file;
+ }
+
+ @Override public String protocol() {
+ return file != null ? "file" : url.getProtocol();
+ }
+
+ @Override public String path() {
+ if (file != null) {
+ return file.getPath();
+ }
+ try {
+ // Decode %20 and friends
+ return url.toURI().getSchemeSpecificPart();
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException("Unable to convert URL " + url + " to URI", e);
+ }
+ }
+
+ @Override public Reader reader() throws IOException {
+ final InputStream is;
+ if (path().endsWith(".gz")) {
+ final InputStream fis = openStream();
+ is = new GZIPInputStream(fis);
+ } else {
+ is = openStream();
+ }
+ return new InputStreamReader(is, StandardCharsets.UTF_8);
+ }
+
+ @Override public InputStream openStream() throws IOException {
+ if (file != null) {
+ return new FileInputStream(file);
+ } else {
+ return url.openStream();
+ }
+ }
+
+ @Override public Source trim(String suffix) {
+ Source x = trimOrNull(suffix);
+ return x == null ? this : x;
+ }
+
+ @Override public Source trimOrNull(String suffix) {
+ if (!urlGenerated) {
+ final String s = Sources.trimOrNull(url.toExternalForm(), suffix);
+ return s == null ? null : Sources.url(s);
+ } else {
+ final String s = Sources.trimOrNull(fileNonNull().getPath(), suffix);
+ return s == null ? null : of(new File(s));
+ }
+ }
+
+ @Override public Source append(Source child) {
+ if (isFile(child)) {
+ if (child.file().isAbsolute()) {
+ return child;
+ }
+ } else {
+ try {
+ URI uri = child.url().toURI();
+ if (!uri.isOpaque()) {
+ // The URL is "absolute" (it starts with a slash)
+ return child;
+ }
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException("Unable to convert URL " + child.url() + " to URI", e);
+ }
+ }
+ String path = child.path();
+ if (!urlGenerated) {
+ String encodedPath = new File(".").toURI().relativize(new File(path).toURI())
+ .getRawSchemeSpecificPart();
+ return Sources.url(url + "/" + encodedPath);
+ } else {
+ return Sources.file(file, path);
+ }
+ }
+
+ @Override public Source relative(Source parent) {
+ if (isFile(parent)) {
+ if (isFile(this)
+ && fileNonNull().getPath().startsWith(parent.file().getPath())) {
+ String rest = fileNonNull().getPath().substring(parent.file().getPath().length());
+ if (rest.startsWith(File.separator)) {
+ return Sources.file(null, rest.substring(File.separator.length()));
+ }
+ }
+ return this;
+ } else {
+ if (!isFile(this)) {
+ String rest = Sources.trimOrNull(url.toExternalForm(),
+ parent.url().toExternalForm());
+ if (rest != null
+ && rest.startsWith("/")) {
+ return Sources.file(null, rest.substring(1));
+ }
+ }
+ return this;
+ }
+ }
+ }
+}
+
+// End Sources.java
diff --git a/gradle.properties b/gradle.properties
index 1e3db05..533f7a5 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -68,7 +68,7 @@ httpcore.version=4.4.11
jackson.version=2.10.0
jcip-annotations.version=1.0-1
jcommander.version=1.72
-jetty.version=9.4.31.v20200723
+jetty.version=9.4.42.v20210604
junit.version=4.12
kerby.version=1.1.1
mockito.version=2.23.4
diff --git a/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java b/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java
index f883b87..c094fc8 100644
--- a/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java
+++ b/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java
@@ -209,8 +209,11 @@ public class HttpServer {
throw new RuntimeException("Server is already started");
}
- final QueuedThreadPool threadPool = new QueuedThreadPool();
- threadPool.setDaemon(true);
+ final SubjectPreservingPrivilegedThreadFactory subjectPreservingPrivilegedThreadFactory =
+ new SubjectPreservingPrivilegedThreadFactory();
+ //The constructor parameters are the Jetty defaults, except for the ThreadFactory
+ final QueuedThreadPool threadPool = new QueuedThreadPool(200, 8, 60000, -1, null, null,
+ subjectPreservingPrivilegedThreadFactory);
server = new Server(threadPool);
server.manage(threadPool);
diff --git a/server/src/main/java/org/apache/calcite/avatica/server/SubjectPreservingPrivilegedThreadFactory.java b/server/src/main/java/org/apache/calcite/avatica/server/SubjectPreservingPrivilegedThreadFactory.java
new file mode 100644
index 0000000..7a6d6b6
--- /dev/null
+++ b/server/src/main/java/org/apache/calcite/avatica/server/SubjectPreservingPrivilegedThreadFactory.java
@@ -0,0 +1,60 @@
+/*
+ * 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.calcite.avatica.server;
+
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.util.concurrent.ThreadFactory;
+import javax.security.auth.Subject;
+
+/**
+ * Encapsulates creating the new Thread in a doPrivileged and a doAs call.
+ * The doPrivilieged block is taken from Jetty, and prevents some classloader leak isses.
+ * Saving the subject, and creating the Thread in the inner doAs call works around
+ * doPriviliged resetting the kerberos subject, which breaks SPNEGO authentication.
+ *
+ * Also sets the daemon flag and name for the Thread, as the QueuedThreadPool parameters are
+ * not applied with a custom ThreadFactory.
+ *
+ * see https://www.ibm.com/docs/en/was-zos/8.5.5\\
+ * ?topic=service-java-authentication-authorization-authorization
+ */
+class SubjectPreservingPrivilegedThreadFactory implements ThreadFactory {
+
+ /**
+ * @param Runnable object for the thread
+ * @return a new thread, protected from classloader pinning, but keeping the current Subject
+ */
+ public Thread newThread(Runnable runnable) {
+ Subject subject = Subject.getSubject(AccessController.getContext());
+ return AccessController.doPrivileged(new PrivilegedAction<Thread>() {
+ @Override public Thread run() {
+ return Subject.doAs(subject, new PrivilegedAction<Thread>() {
+ @Override public Thread run() {
+ Thread thread = new Thread(runnable);
+ thread.setDaemon(true);
+ thread.setName("avatica_qtp" + hashCode() + "-" + thread.getId());
+ thread.setContextClassLoader(getClass().getClassLoader());
+ return thread;
+ }
+ });
+ }
+ });
+ }
+}
+
+// End SubjectPreservingPrivilegedThreadFactory.java
diff --git a/server/src/test/java/org/apache/calcite/avatica/server/BasicAuthHttpServerTest.java b/server/src/test/java/org/apache/calcite/avatica/server/BasicAuthHttpServerTest.java
index 9f9914a..69c5bf4 100644
--- a/server/src/test/java/org/apache/calcite/avatica/server/BasicAuthHttpServerTest.java
+++ b/server/src/test/java/org/apache/calcite/avatica/server/BasicAuthHttpServerTest.java
@@ -20,12 +20,12 @@ import org.apache.calcite.avatica.ConnectionSpec;
import org.apache.calcite.avatica.jdbc.JdbcMeta;
import org.apache.calcite.avatica.remote.Driver;
import org.apache.calcite.avatica.remote.LocalService;
+import org.apache.calcite.avatica.util.Sources;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
-import java.net.URLDecoder;
import java.util.Properties;
import static org.hamcrest.core.StringContains.containsString;
@@ -44,8 +44,9 @@ public class BasicAuthHttpServerTest extends HttpAuthBase {
private static String url;
@BeforeClass public static void startServer() throws Exception {
- final String userPropertiesFile = URLDecoder.decode(BasicAuthHttpServerTest.class
- .getResource("/auth-users.properties").getFile(), "UTF-8");
+ final String userPropertiesFile = Sources.of(
+ BasicAuthHttpServerTest.class.getResource("/auth-users.properties")).file()
+ .getAbsolutePath();
assertNotNull("Could not find properties file for basic auth users", userPropertiesFile);
// Create a LocalService around HSQLDB
diff --git a/server/src/test/java/org/apache/calcite/avatica/server/DigestAuthHttpServerTest.java b/server/src/test/java/org/apache/calcite/avatica/server/DigestAuthHttpServerTest.java
index a5d8e86..3a013bd 100644
--- a/server/src/test/java/org/apache/calcite/avatica/server/DigestAuthHttpServerTest.java
+++ b/server/src/test/java/org/apache/calcite/avatica/server/DigestAuthHttpServerTest.java
@@ -20,12 +20,12 @@ import org.apache.calcite.avatica.ConnectionSpec;
import org.apache.calcite.avatica.jdbc.JdbcMeta;
import org.apache.calcite.avatica.remote.Driver;
import org.apache.calcite.avatica.remote.LocalService;
+import org.apache.calcite.avatica.util.Sources;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
-import java.net.URLDecoder;
import java.util.Properties;
import static org.hamcrest.core.StringContains.containsString;
@@ -44,8 +44,8 @@ public class DigestAuthHttpServerTest extends HttpAuthBase {
private static String url;
@BeforeClass public static void startServer() throws Exception {
- final String userPropertiesFile = URLDecoder.decode(BasicAuthHttpServerTest.class
- .getResource("/auth-users.properties").getFile(), "UTF-8");
+ final String userPropertiesFile = Sources.of(DigestAuthHttpServerTest.class
+ .getResource("/auth-users.properties")).file().getAbsolutePath();
assertNotNull("Could not find properties file for digest auth users", userPropertiesFile);
// Create a LocalService around HSQLDB
diff --git a/server/src/test/java/org/apache/calcite/avatica/server/HttpAuthBase.java b/server/src/test/java/org/apache/calcite/avatica/server/HttpAuthBase.java
index 6ce0afe..8e7cfce 100644
--- a/server/src/test/java/org/apache/calcite/avatica/server/HttpAuthBase.java
+++ b/server/src/test/java/org/apache/calcite/avatica/server/HttpAuthBase.java
@@ -17,9 +17,8 @@
package org.apache.calcite.avatica.server;
import org.apache.calcite.avatica.ConnectionSpec;
+import org.apache.calcite.avatica.util.Sources;
-import java.io.UnsupportedEncodingException;
-import java.net.URLDecoder;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
@@ -79,15 +78,10 @@ public class HttpAuthBase {
}
static String getHashLoginServicePropertiesString() {
- try {
- final String userPropertiesFile =
- URLDecoder.decode(HttpQueryStringParameterRemoteUserExtractorTest.class
- .getResource("/auth-users.properties").getFile(), "UTF-8");
- assertNotNull("Could not find properties file for basic auth users", userPropertiesFile);
- return userPropertiesFile;
- } catch (UnsupportedEncodingException e) {
- throw new RuntimeException(e);
- }
+ final String userPropertiesFile = Sources.of(HttpAuthBase.class
+ .getResource("/auth-users.properties")).file().getAbsolutePath();
+ assertNotNull("Could not find properties file for basic auth users", userPropertiesFile);
+ return userPropertiesFile;
}
}