From 5f8bb112c4bab2b9a7c26153b2e3d46da28db386 Mon Sep 17 00:00:00 2001 From: Istvan Toth Date: Fri, 11 Jun 2021 09:45:29 +0200 Subject: [PATCH 1/3] CALCITE-4646 Update to Jetty 9.4.42.v20210604 --- gradle.properties | 2 +- .../calcite/avatica/server/HttpServer.java | 7 ++- ...jectPreservingPrivilegedThreadFactory.java | 60 +++++++++++++++++++ .../server/BasicAuthHttpServerTest.java | 6 +- .../server/DigestAuthHttpServerTest.java | 6 +- .../calcite/avatica/server/HttpAuthBase.java | 11 ++-- 6 files changed, 77 insertions(+), 15 deletions(-) create mode 100644 server/src/main/java/org/apache/calcite/avatica/server/SubjectPreservingPrivilegedThreadFactory.java diff --git a/gradle.properties b/gradle.properties index 1e3db0530f..533f7a55c1 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 f883b87bd6..c094fc8fa1 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 @@ protected void internalStart() { 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 0000000000..7a6d6b6841 --- /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() { + @Override public Thread run() { + return Subject.doAs(subject, new PrivilegedAction() { + @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 9f9914ad85..fc6993dd00 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 @@ -25,7 +25,7 @@ import org.junit.BeforeClass; import org.junit.Test; -import java.net.URLDecoder; +import java.io.File; import java.util.Properties; import static org.hamcrest.core.StringContains.containsString; @@ -44,8 +44,8 @@ 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 = (new File(BasicAuthHttpServerTest.class + .getResource("/auth-users.properties").toURI())).getPath(); 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 a5d8e8667b..67fee72249 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 @@ -25,7 +25,7 @@ import org.junit.BeforeClass; import org.junit.Test; -import java.net.URLDecoder; +import java.io.File; 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 = (new File(BasicAuthHttpServerTest.class + .getResource("/auth-users.properties").toURI())).getPath(); 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 6ce0afefe2..84a3e5e571 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 @@ -18,8 +18,8 @@ import org.apache.calcite.avatica.ConnectionSpec; -import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; +import java.io.File; +import java.net.URISyntaxException; import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; @@ -80,12 +80,11 @@ void readWriteData(String url, String tableName, Properties props) throws Except static String getHashLoginServicePropertiesString() { try { - final String userPropertiesFile = - URLDecoder.decode(HttpQueryStringParameterRemoteUserExtractorTest.class - .getResource("/auth-users.properties").getFile(), "UTF-8"); + final String userPropertiesFile = (new File(BasicAuthHttpServerTest.class + .getResource("/auth-users.properties").toURI())).getPath(); assertNotNull("Could not find properties file for basic auth users", userPropertiesFile); return userPropertiesFile; - } catch (UnsupportedEncodingException e) { + } catch (URISyntaxException e) { throw new RuntimeException(e); } } From 77a3ceeac0cc9085333fb77195067c334f48f143 Mon Sep 17 00:00:00 2001 From: Istvan Toth Date: Mon, 5 Jul 2021 07:34:55 +0200 Subject: [PATCH 2/3] copy most of org.apache.calcite.util.Sources to Avatica, and use it in the test Change-Id: I04d13b49eba395b2ae41e31b962d1bd4c3ed5cc2 --- .../apache/calcite/avatica/util/Source.java | 72 +++++ .../apache/calcite/avatica/util/Sources.java | 274 ++++++++++++++++++ .../server/BasicAuthHttpServerTest.java | 7 +- 3 files changed, 350 insertions(+), 3 deletions(-) create mode 100644 core/src/main/java/org/apache/calcite/avatica/util/Source.java create mode 100644 core/src/main/java/org/apache/calcite/avatica/util/Sources.java 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 0000000000..f7dcfe00ed --- /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. + * + *

For example, + *

    + *
  • source("/foo").append(source("bar")) + * returns source("/foo/bar") + *
  • source("/foo").append(source("/bar")) + * returns source("/bar") + * because "/bar" was already absolute + *
+ */ + Source append(Source child); + + /** Returns a relative source, if this source is a child of a given base. + * + *

For example, + *

    + *
  • source("/foo/bar").relative(source("/foo")) + * returns source("bar") + *
  • source("/baz/bar").relative(source("/foo")) + * returns source("/baz/bar") + *
+ */ + 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 0000000000..46762ea252 --- /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/server/src/test/java/org/apache/calcite/avatica/server/BasicAuthHttpServerTest.java b/server/src/test/java/org/apache/calcite/avatica/server/BasicAuthHttpServerTest.java index fc6993dd00..69c5bf4b7b 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.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.io.File; 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 = (new File(BasicAuthHttpServerTest.class - .getResource("/auth-users.properties").toURI())).getPath(); + 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 From 0294623f26493197d431a3b962072256efc3ab5f Mon Sep 17 00:00:00 2001 From: Istvan Toth Date: Mon, 5 Jul 2021 14:59:41 +0200 Subject: [PATCH 3/3] load auth-users.properties via Sources class in each test Change-Id: I9d41880c427ce359e3e284f1f94d9aa877d33234 --- .../avatica/server/DigestAuthHttpServerTest.java | 6 +++--- .../calcite/avatica/server/HttpAuthBase.java | 15 +++++---------- 2 files changed, 8 insertions(+), 13 deletions(-) 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 67fee72249..3a013bd324 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.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.io.File; 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 = (new File(BasicAuthHttpServerTest.class - .getResource("/auth-users.properties").toURI())).getPath(); + 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 84a3e5e571..8e7cfcec6c 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.File; -import java.net.URISyntaxException; import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; @@ -79,14 +78,10 @@ void readWriteData(String url, String tableName, Properties props) throws Except } static String getHashLoginServicePropertiesString() { - try { - final String userPropertiesFile = (new File(BasicAuthHttpServerTest.class - .getResource("/auth-users.properties").toURI())).getPath(); - assertNotNull("Could not find properties file for basic auth users", userPropertiesFile); - return userPropertiesFile; - } catch (URISyntaxException 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; } }