+ * {@link AuthenticationFilter} reports its reason through + * {@link javax.servlet.http.HttpServletResponse#sendError}, which used to + * reach the caller in the HTTP reason phrase. Jetty 12 does not put a reason + * phrase on the wire, so {@link HttpURLConnection#getResponseMessage()} now + * only ever returns the canonical text for the status code and the reason is + * in the body. Read the body, and fall back to the phrase when it is empty. + *
+ * A JSON body is left alone. That is the envelope + * HttpExceptionUtils#createServletExceptionResponse writes, and a refusal + * that carries it never carried a reason phrase of its own - it is sent with + * setStatus, not sendError, so the phrase was the canonical text for the + * status code before Jetty 12 and still is. Quoting the envelope back as + * free text would replace a readable "Forbidden" with a line of JSON, and + * the callers that want what is inside it parse it with + * HttpExceptionUtils#validateResponse instead. + */ + private static String responseDetail(HttpURLConnection conn) { + if (isJson(conn.getContentType())) { + return responsePhrase(conn); + } + try (InputStream es = conn.getErrorStream()) { + if (es != null) { + byte[] body = new byte[MAX_RESPONSE_DETAIL_BYTES]; + int read = 0, n; + while (read < body.length + && (n = es.read(body, read, body.length - read)) != -1) { + read += n; + } + String text = toPlainText( + new String(body, 0, read, StandardCharsets.UTF_8)); + if (!text.isEmpty()) { + return text; + } + } + } catch (IOException ex) { + // nothing to add: fall through to the reason phrase + } + return responsePhrase(conn); + } + + /** + * Whether the content type names the JSON error envelope. The header can + * carry parameters - "application/json; charset=utf-8" - so this matches a + * prefix rather than the whole value. + */ + private static boolean isJson(String contentType) { + return contentType != null + && contentType.trim().toLowerCase().startsWith(APPLICATION_JSON_MIME); + } + + /** + * Reduces a response body to something readable in a one-line message. A + * container renders sendError as an HTML page, so the reason arrives buried + * in markup. + *
+ * Kept in step with {@code HttpExceptionUtils.toPlainText}, which does the + * same job for the same bodies one module up. The duplication is forced: + * hadoop-common depends on this module, not the other way round. + */ + private static String toPlainText(String body) { + String text = body; + if (text.indexOf('<') >= 0) { + text = text.replaceAll("(?s)<(script|style)\\b.*?", " ") + .replaceAll("(?s)<[^>]*>", " "); + } + text = text.replace("<", "<").replace(">", ">") + .replace(""", "\"").replace("'", "'") + .replace("&", "&"); + return text.replaceAll("\\s+", " ").trim(); + } + + /** The reason phrase, or null if it cannot be read. */ + private static String responsePhrase(HttpURLConnection conn) { + try { + return conn.getResponseMessage(); + } catch (IOException ex) { + return null; } } diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java index 7cc70c493c0f66..3549ef1ed9c42a 100644 --- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java @@ -621,8 +621,11 @@ && getMaxInactiveInterval() > 0) { KerberosAuthenticator.WWW_AUTHENTICATE.toLowerCase()))) { errCode = HttpServletResponse.SC_FORBIDDEN; } - // After Jetty 9.4.21, sendError() no longer allows a custom message. - // use setStatus() to set a custom message. + // The reason phrase is not a place to put this any more. Jetty 12 + // stores what ee8's Response.setStatusWithReason is given and never + // writes it to the wire, and setStatus(int, String) has ignored its + // message since Servlet 3.0 deprecated it. sendError puts the detail + // in the response body, which is where a client can still read it. String reason; if (authenticationEx == null) { reason = "Authentication required"; @@ -630,7 +633,6 @@ && getMaxInactiveInterval() > 0) { reason = authenticationEx.getMessage(); } - httpResponse.setStatus(errCode, reason); httpResponse.sendError(errCode, reason); } } diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationToken.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationToken.java index 8295fe173f4b1a..68da16580ec944 100644 --- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationToken.java +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationToken.java @@ -18,12 +18,10 @@ import java.security.Principal; -import javax.servlet.http.HttpServletRequest; - /** * The {@link AuthenticationToken} contains information about an authenticated * HTTP client and doubles as the {@link Principal} to be returned by - * authenticated {@link HttpServletRequest}s + * authenticated HTTP requests. *
* The token can be serialized/deserialized to and from a string as it is sent * and received in HTTP client responses and requests as a HTTP cookie (this is diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/JWTRedirectAuthenticationHandler.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/JWTRedirectAuthenticationHandler.java index 2dcb60836b5e43..5fba0af1bf2a58 100644 --- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/JWTRedirectAuthenticationHandler.java +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/JWTRedirectAuthenticationHandler.java @@ -26,6 +26,7 @@ import java.util.Properties; import java.text.ParseException; +import java.security.cert.CertificateException; import java.security.interfaces.RSAPublicKey; import org.apache.hadoop.classification.VisibleForTesting; @@ -125,7 +126,11 @@ public void init(Properties config) throws ServletException { throw new ServletException( "Public key for signature validation must be provisioned."); } - publicKey = CertificateUtil.parseRSAPublicKey(pemPublicKey); + try { + publicKey = CertificateUtil.toRSAPublicKey(pemPublicKey); + } catch (CertificateException ce) { + throw new ServletException(ce.getMessage(), ce); + } } // setup the list of valid audiences for token validation String auds = config.getProperty(EXPECTED_JWT_AUDIENCES); diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/CertificateUtil.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/CertificateUtil.java index f25602c67d4a31..13840948ab349f 100644 --- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/CertificateUtil.java +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/CertificateUtil.java @@ -38,8 +38,37 @@ public class CertificateUtil { * - the pem encoding from config without the header and footer * @return RSAPublicKey the RSA public key * @throws ServletException thrown if a processing error occurred + * @deprecated use {@link #toRSAPublicKey(String)}, which reports the + * {@link CertificateException} it is really raising instead of + * wrapping it in a servlet type. This method is kept so that + * existing callers still compile and behave as before, and will + * be removed with the move to the jakarta servlet namespace. */ - public static RSAPublicKey parseRSAPublicKey(String pem) throws ServletException { + @Deprecated + public static RSAPublicKey parseRSAPublicKey(String pem) + throws ServletException { + try { + return toRSAPublicKey(pem); + } catch (CertificateException ce) { + // Report the exception toRSAPublicKey wrapped, not the wrapper, so that + // the cause a caller sees is the one this method has always reported: + // the CertificateException the parse itself raised. Wrapping the wrapper + // would add a level to the chain that was not there before. + Throwable cause = ce.getCause() == null ? ce : ce.getCause(); + throw new ServletException(ce.getMessage(), cause); + } + } + + /** + * Gets an RSAPublicKey from the provided PEM encoding. + * + * @param pem + * - the pem encoding from config without the header and footer + * @return RSAPublicKey the RSA public key + * @throws CertificateException thrown if the PEM could not be parsed + */ + public static RSAPublicKey toRSAPublicKey(String pem) + throws CertificateException { String fullPem = PEM_HEADER + pem + PEM_FOOTER; PublicKey key = null; try { @@ -57,7 +86,7 @@ public static RSAPublicKey parseRSAPublicKey(String pem) throws ServletException } else { message = "CertificateException - PEM may be corrupt"; } - throw new ServletException(message, ce); + throw new CertificateException(message, ce); } return (RSAPublicKey) key; } diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/FileSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/FileSignerSecretProvider.java index 2a8a712b595bac..66bd181771a9a1 100644 --- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/FileSignerSecretProvider.java +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/FileSignerSecretProvider.java @@ -17,7 +17,6 @@ import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.security.authentication.server.AuthenticationFilter; -import javax.servlet.ServletContext; import java.io.*; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -37,7 +36,7 @@ public class FileSignerSecretProvider extends SignerSecretProvider { public FileSignerSecretProvider() {} @Override - public void init(Properties config, ServletContext servletContext, + public void initialize(Properties config, SecretProviderContext context, long tokenValidity) throws Exception { String signatureSecretFile = config.getProperty( diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java index ca95272cf9fd62..9edc3145c4a02c 100644 --- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java @@ -17,7 +17,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import javax.servlet.ServletContext; import org.apache.hadoop.classification.VisibleForTesting; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -61,12 +60,12 @@ public RolloverSignerSecretProvider() { * and starts the scheduler for the rollover to run at an interval of * tokenValidity. * @param config configuration properties - * @param servletContext servlet context + * @param context the attribute store to initialize against * @param tokenValidity The amount of time a token is valid for * @throws Exception thrown if an error occurred */ @Override - public void init(Properties config, ServletContext servletContext, + public void initialize(Properties config, SecretProviderContext context, long tokenValidity) throws Exception { initSecrets(generateNewSecret(), null); startScheduler(tokenValidity, tokenValidity); diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SecretProviderContext.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SecretProviderContext.java new file mode 100644 index 00000000000000..b002ab0c047303 --- /dev/null +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SecretProviderContext.java @@ -0,0 +1,52 @@ +/** + * Licensed 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. See accompanying LICENSE file. + */ +package org.apache.hadoop.security.authentication.util; + +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; + +/** + * The attribute store a {@link SignerSecretProvider} is initialized against. + *
+ * A SignerSecretProvider that has to share an object with the rest of the + * application it is embedded in - as {@link ZKSignerSecretProvider} shares its + * CuratorFramework client - reads and writes that object here. When the + * provider is running inside a servlet container the attributes are those of + * the ServletContext, so the sharing is unchanged; see + * {@link org.apache.hadoop.security.authentication.server.AuthenticationFilter}. + *
+ * This interface exists so that a SignerSecretProvider need not name a servlet + * type. Implementations of it that are backed by a ServletContext do, but the + * providers themselves stay independent of the servlet API and so of which + * servlet namespace - javax or jakarta - the container provides. + */ +@InterfaceStability.Unstable +@InterfaceAudience.Private +public interface SecretProviderContext { + + /** + * Returns the attribute stored under the given name, or null if there is + * none. + * @param name the attribute name + * @return the attribute value, or null + */ + Object getAttribute(String name); + + /** + * Stores an attribute under the given name, replacing any previous value. + * @param name the attribute name + * @param value the attribute value + */ + void setAttribute(String name, Object value); +} diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ServletSecretProviderContext.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ServletSecretProviderContext.java new file mode 100644 index 00000000000000..317eeb541453e9 --- /dev/null +++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ServletSecretProviderContext.java @@ -0,0 +1,117 @@ +/** + * Licensed 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. See accompanying LICENSE file. + */ +package org.apache.hadoop.security.authentication.util; + +import java.util.HashMap; +import java.util.Map; +import javax.servlet.ServletContext; +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.classification.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Presents a ServletContext to a {@link SignerSecretProvider} as a + * {@link SecretProviderContext}. + *
+ * Reads and writes go straight through to the ServletContext, so an object a + * provider shares this way - the CuratorFramework client of + * {@link ZKSignerSecretProvider} - is still a ServletContext attribute under + * the same name, and is still found there by everything that looks for it, + * including DelegationTokenAuthenticationFilter in hadoop-common. + *
+ * This class is the one place in the provider hierarchy that names a servlet + * type, and it goes away with the deprecated + * {@link SignerSecretProvider#init(java.util.Properties, ServletContext, long)} + * it exists to support. + */ +@InterfaceStability.Unstable +@InterfaceAudience.Private +final class ServletSecretProviderContext implements SecretProviderContext { + + private static final Logger LOG = + LoggerFactory.getLogger(ServletSecretProviderContext.class); + + private final ServletContext servletContext; + + private ServletSecretProviderContext(ServletContext servletContext) { + this.servletContext = servletContext; + } + + /** + * Returns a context backed by the given ServletContext, or one backed by a + * map of its own when there is no ServletContext. + *
+ * Most callers of the deprecated init pass null - only AuthenticationFilter
+ * ever passes a real context - and a provider that does not use the store is
+ * unaffected either way. Handing back a store rather than null keeps a
+ * provider that does use it from having to check.
+ *
+ * @param servletContext the servlet context, or null
+ * @return an attribute store, never null
+ */
+ static SecretProviderContext of(ServletContext servletContext) {
+ return servletContext == null
+ ? new MapSecretProviderContext()
+ : new ServletSecretProviderContext(servletContext);
+ }
+
+ @Override
+ public Object getAttribute(String name) {
+ return servletContext.getAttribute(name);
+ }
+
+ @Override
+ public void setAttribute(String name, Object value) {
+ servletContext.setAttribute(name, value);
+ }
+
+ /**
+ * The store used when there is no ServletContext to write through to.
+ * Attributes live as long as the provider does and are seen by nothing else,
+ * which is what passing a null ServletContext already meant.
+ */
+ @VisibleForTesting
+ static final class MapSecretProviderContext
+ implements SecretProviderContext {
+
+ private final Map
+ * A provider only writes here to share an object with the rest of the
+ * application - the CuratorFramework client of
+ * {@link ZKSignerSecretProvider} is the one case in this tree. There is
+ * nothing to share it with: this store is private to the provider, so a
+ * second provider in the same JVM builds a second client rather than
+ * finding this one. Before there was a store at all a null ServletContext
+ * threw, which was at least loud; this keeps it visible.
+ */
+ @Override
+ public void setAttribute(String name, Object value) {
+ LOG.warn("SignerSecretProvider was initialized without a ServletContext,"
+ + " so the attribute {} is being stored in a map private to this"
+ + " provider. Anything that expects to share it - including another"
+ + " provider in this JVM - will not find it there.", name);
+ attributes.put(name, value);
+ }
+ }
+}
diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java
index e937862458e0d0..ff5acc210f7805 100644
--- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/SignerSecretProvider.java
@@ -24,20 +24,77 @@
* do more complicated things in the backend.
* See the RolloverSignerSecretProvider class for an implementation that
* supports rolling over the secret at a regular interval.
+ *
+ * Implementations should override
+ * {@link #initialize(Properties, SecretProviderContext, long)}, which names no
+ * servlet type. The older {@link #init(Properties, ServletContext, long)} is
+ * still the entry point callers use and is still honoured when an
+ * implementation overrides it, so providers written against it keep working
+ * unchanged; it is deprecated and will be removed with the move to the jakarta
+ * servlet namespace.
+ *
+ * One consequence for code that reflects over these classes: the providers
+ * shipped here override
+ * {@link #initialize(Properties, SecretProviderContext, long)} rather than
+ * {@link #init(Properties, ServletContext, long)}, so init is no longer among
+ * their declared methods. Calls and overrides are unaffected - init is
+ * inherited from this class and resolves as it always did - but
+ * Class#getDeclaredMethod("init", ...) on a subclass such as
+ * {@link FileSignerSecretProvider} now raises NoSuchMethodException where it
+ * used to succeed. Class#getMethod, which searches superclasses, still finds
+ * it.
*/
@InterfaceStability.Unstable
@InterfaceAudience.Private
public abstract class SignerSecretProvider {
/**
- * Initialize the SignerSecretProvider
+ * Initialize the SignerSecretProvider.
+ *
+ * The default implementation adapts the ServletContext to a
+ * {@link SecretProviderContext} and calls
+ * {@link #initialize(Properties, SecretProviderContext, long)}, so a provider
+ * that overrides only that method is initialized correctly through this
+ * entry point. A provider that overrides this method instead is called
+ * directly, as before.
+ *
* @param config configuration properties
* @param servletContext servlet context
* @param tokenValidity The amount of time a token is valid for
* @throws Exception thrown if an error occurred
+ * @deprecated override
+ * {@link #initialize(Properties, SecretProviderContext, long)},
+ * which does not name a servlet type and so is unaffected by
+ * which servlet namespace the container provides.
*/
- public abstract void init(Properties config, ServletContext servletContext,
- long tokenValidity) throws Exception;
+ @Deprecated
+ public void init(Properties config, ServletContext servletContext,
+ long tokenValidity) throws Exception {
+ initialize(config, ServletSecretProviderContext.of(servletContext),
+ tokenValidity);
+ }
+
+ /**
+ * Initialize the SignerSecretProvider against an attribute store.
+ *
+ * The default implementation throws, because a provider has to implement one
+ * of the two initialization methods. It is never reached by a provider that
+ * overrides the deprecated
+ * {@link #init(Properties, ServletContext, long)}, since callers go through
+ * that method and its override does not delegate here.
+ *
+ * @param config configuration properties
+ * @param context the attribute store to initialize against
+ * @param tokenValidity The amount of time a token is valid for
+ * @throws Exception thrown if an error occurred
+ */
+ public void initialize(Properties config, SecretProviderContext context,
+ long tokenValidity) throws Exception {
+ throw new UnsupportedOperationException(getClass().getName()
+ + " implements neither initialize(Properties, SecretProviderContext,"
+ + " long) nor the deprecated init(Properties, ServletContext, long)");
+ }
+
/**
* Will be called on shutdown; subclasses should perform any cleanup here.
*/
diff --git a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
index b0604c85c39f42..853cffbfa9e412 100644
--- a/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/ZKSignerSecretProvider.java
@@ -18,7 +18,6 @@
import java.security.SecureRandom;
import java.util.Properties;
import java.util.Random;
-import javax.servlet.ServletContext;
import org.apache.curator.framework.CuratorFramework;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
@@ -100,9 +99,11 @@ public class ZKSignerSecretProvider extends RolloverSignerSecretProvider {
CONFIG_PREFIX + "disconnect.on.shutdown";
/**
- * Constant for the ServletContext attribute that can be used for providing a
- * custom CuratorFramework client. If set ZKSignerSecretProvider will use this
- * Curator client instead of creating a new one. The providing class is
+ * Constant for the {@link SecretProviderContext} attribute that can be used
+ * for providing a custom CuratorFramework client. In a servlet container that
+ * is the ServletContext attribute of the same name, so the way it is set is
+ * unchanged. If set ZKSignerSecretProvider will use this Curator client
+ * instead of creating a new one. The providing class is
* responsible for creating and configuring the Curator client (including
* security and ACLs) in this case.
*/
@@ -159,16 +160,16 @@ public ZKSignerSecretProvider(long seed) {
}
@Override
- public void init(Properties config, ServletContext servletContext,
+ public void initialize(Properties config, SecretProviderContext context,
long tokenValidity) throws Exception {
- Object curatorClientObj = servletContext.getAttribute(
+ Object curatorClientObj = context.getAttribute(
ZOOKEEPER_SIGNER_SECRET_PROVIDER_CURATOR_CLIENT_ATTRIBUTE);
if (curatorClientObj != null
&& curatorClientObj instanceof CuratorFramework) {
client = (CuratorFramework) curatorClientObj;
} else {
client = createCuratorClient(config);
- servletContext.setAttribute(
+ context.setAttribute(
ZOOKEEPER_SIGNER_SECRET_PROVIDER_CURATOR_CLIENT_ATTRIBUTE, client);
}
this.tokenValidity = tokenValidity;
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/AuthenticatorTestCase.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/AuthenticatorTestCase.java
index 14538b873c803c..c6137b4ee08cb9 100644
--- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/AuthenticatorTestCase.java
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/AuthenticatorTestCase.java
@@ -35,9 +35,9 @@
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
-import org.eclipse.jetty.servlet.FilterHolder;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jetty.ee8.servlet.FilterHolder;
+import org.eclipse.jetty.ee8.servlet.ServletContextHandler;
+import org.eclipse.jetty.ee8.servlet.ServletHolder;
import javax.servlet.DispatcherType;
import javax.servlet.FilterConfig;
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/TestPseudoAuthenticator.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/TestPseudoAuthenticator.java
index 43f38f2594e263..b944857656c872 100644
--- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/TestPseudoAuthenticator.java
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/TestPseudoAuthenticator.java
@@ -20,9 +20,12 @@
import org.apache.hadoop.security.authentication.server.PseudoAuthenticationHandler;
import org.junit.jupiter.api.Test;
+import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.URL;
+import java.nio.charset.StandardCharsets;
import java.util.Properties;
+import java.util.Scanner;
public class TestPseudoAuthenticator {
@@ -67,7 +70,9 @@ public void testAnonymousDisallowed() throws Exception {
conn.connect();
assertEquals(HttpURLConnection.HTTP_UNAUTHORIZED, conn.getResponseCode());
assertTrue(conn.getHeaderFields().containsKey("WWW-Authenticate"));
- assertEquals("Authentication required", conn.getResponseMessage());
+ // Jetty 12 sends the canonical reason phrase whatever the server sets,
+ // so the detail is read out of the body instead.
+ assertTrue(readBody(conn).contains("Authentication required"));
} finally {
auth.stop();
}
@@ -105,4 +110,17 @@ public void testAuthenticationAnonymousDisallowedWithPost() throws Exception {
auth._testAuthentication(new PseudoAuthenticator(), true);
}
+ private static String readBody(HttpURLConnection conn) {
+ InputStream in = conn.getErrorStream() != null
+ ? conn.getErrorStream() : null;
+ if (in == null) {
+ return "";
+ }
+ try (Scanner scanner = new Scanner(in, StandardCharsets.UTF_8.name())) {
+ // \A matches only at the start of input, so the whole body is one token.
+ scanner.useDelimiter("\\A");
+ return scanner.hasNext() ? scanner.next() : "";
+ }
+ }
+
}
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java
index 9d857640bcf482..ed5587d7a699d4 100644
--- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/StringSignerSecretProvider.java
@@ -15,7 +15,6 @@
import java.nio.charset.StandardCharsets;
import java.util.Properties;
-import javax.servlet.ServletContext;
import org.apache.hadoop.classification.VisibleForTesting;
import org.apache.hadoop.classification.InterfaceStability;
@@ -34,7 +33,7 @@ class StringSignerSecretProvider extends SignerSecretProvider {
public StringSignerSecretProvider() {}
@Override
- public void init(Properties config, ServletContext servletContext,
+ public void initialize(Properties config, SecretProviderContext context,
long tokenValidity) throws Exception {
String signatureSecret = config.getProperty(
AuthenticationFilter.SIGNATURE_SECRET, null);
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestCertificateUtil.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestCertificateUtil.java
index 0580bac9053be3..92851654e4e041 100644
--- a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestCertificateUtil.java
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestCertificateUtil.java
@@ -18,10 +18,12 @@
package org.apache.hadoop.security.authentication.util;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import java.security.cert.CertificateException;
import java.security.interfaces.RSAPublicKey;
import javax.servlet.ServletException;
@@ -45,9 +47,9 @@ public void testInvalidPEMWithHeaderAndFooter() throws Exception {
+ "9aTyR+HGHCfvwoCegc9rAVw/DLaRriSO/jnEXzYK6XLVKH+hx5UXrJ7Oyc7JjZUc3g9kCWORThCX"
+ "Mzc1xA==" + "\n-----END CERTIFICATE-----";
try {
- CertificateUtil.parseRSAPublicKey(pem);
- fail("Should not have thrown ServletException");
- } catch (ServletException se) {
+ CertificateUtil.toRSAPublicKey(pem);
+ fail("Should not have thrown CertificateException");
+ } catch (CertificateException se) {
assertTrue(se.getMessage().contains("PEM header"));
}
}
@@ -66,9 +68,9 @@ public void testCorruptPEM() throws Exception {
+ "9aTyR+HGHCfvwoCegc9rAVw/DLaRriSO/jnEXzYK6XLVKH+hx5UXrJ7Oyc7JjZUc3g9kCWORThCX"
+ "Mzc1xA++";
try {
- CertificateUtil.parseRSAPublicKey(pem);
- fail("Should not have thrown ServletException");
- } catch (ServletException se) {
+ CertificateUtil.toRSAPublicKey(pem);
+ fail("Should not have thrown CertificateException");
+ } catch (CertificateException se) {
assertTrue(se.getMessage().contains("corrupt"));
}
}
@@ -87,12 +89,62 @@ public void testValidPEM() throws Exception {
+ "9aTyR+HGHCfvwoCegc9rAVw/DLaRriSO/jnEXzYK6XLVKH+hx5UXrJ7Oyc7JjZUc3g9kCWORThCX"
+ "Mzc1xA==";
try {
- RSAPublicKey pk = CertificateUtil.parseRSAPublicKey(pem);
+ RSAPublicKey pk = CertificateUtil.toRSAPublicKey(pem);
assertNotNull(pk);
assertEquals("RSA", pk.getAlgorithm());
+ } catch (CertificateException se) {
+ fail("Should not have thrown CertificateException");
+ }
+ }
+
+ /**
+ * The deprecated entry point still reports a ServletException, so callers
+ * written against it keep compiling and keep catching what they caught
+ * before.
+ */
+ @Test
+ @SuppressWarnings("deprecation")
+ public void testDeprecatedEntryPointStillThrowsServletException() {
+ String pem = "not a certificate";
+ try {
+ CertificateUtil.parseRSAPublicKey(pem);
+ fail("Should not have thrown ServletException");
} catch (ServletException se) {
+ assertTrue(se.getMessage().contains("corrupt"));
+ assertTrue(se.getCause() instanceof CertificateException);
+ }
+ }
+
+ /**
+ * The deprecated entry point reports the CertificateException the parse
+ * raised, not the one toRSAPublicKey wraps it in. Routing through the new
+ * method must not lengthen the chain a caller walks.
+ */
+ @Test
+ @SuppressWarnings("deprecation")
+ public void testDeprecatedEntryPointDoesNotDeepenTheCauseChain() {
+ try {
+ CertificateUtil.parseRSAPublicKey("not a certificate");
fail("Should not have thrown ServletException");
+ } catch (ServletException se) {
+ Throwable cause = se.getCause();
+ assertNotNull(cause, "the ServletException should carry a cause");
+ assertTrue(cause instanceof CertificateException,
+ "expected a CertificateException, got " + cause.getClass());
+ assertFalse(cause.getCause() instanceof CertificateException,
+ "the wrapper toRSAPublicKey adds must not appear in the chain;"
+ + " chain was " + chainOf(se));
}
}
+ private static String chainOf(Throwable t) {
+ StringBuilder sb = new StringBuilder();
+ for (Throwable c = t; c != null; c = c.getCause()) {
+ if (sb.length() > 0) {
+ sb.append(" -> ");
+ }
+ sb.append(c.getClass().getSimpleName());
+ }
+ return sb.toString();
+ }
}
diff --git a/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSignerSecretProviderCompatibility.java b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSignerSecretProviderCompatibility.java
new file mode 100644
index 00000000000000..2a515889da46b0
--- /dev/null
+++ b/hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestSignerSecretProviderCompatibility.java
@@ -0,0 +1,228 @@
+/**
+ * Licensed 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. See accompanying LICENSE file.
+ */
+package org.apache.hadoop.security.authentication.util;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+
+import javax.servlet.ServletContext;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * A SignerSecretProvider written against the deprecated
+ * {@link SignerSecretProvider#init(Properties, ServletContext, long)} must keep
+ * working without being recompiled, because implementations of it live outside
+ * this tree - signer.secret.provider takes a classname.
+ *
+ * These tests pin that contract. They are the reason
+ * {@link SignerSecretProvider#init(Properties, ServletContext, long)} is a
+ * concrete bridge rather than a changed abstract method, and they go when it
+ * does.
+ */
+@SuppressWarnings("deprecation")
+public class TestSignerSecretProviderCompatibility {
+
+ private static final byte[] SECRET =
+ "secret".getBytes(StandardCharsets.UTF_8);
+
+ /**
+ * A provider as it would have been written before SecretProviderContext
+ * existed: it overrides init and takes a ServletContext.
+ */
+ private static final class LegacyProvider extends SignerSecretProvider {
+ private ServletContext seen;
+ private boolean initCalled;
+
+ @Override
+ public void init(Properties config, ServletContext servletContext,
+ long tokenValidity) {
+ this.seen = servletContext;
+ this.initCalled = true;
+ }
+
+ @Override
+ public byte[] getCurrentSecret() {
+ return SECRET;
+ }
+
+ @Override
+ public byte[][] getAllSecrets() {
+ return new byte[][]{SECRET};
+ }
+ }
+
+ /**
+ * The same, extending RolloverSignerSecretProvider and chaining to super as
+ * such a provider is expected to.
+ */
+ private static final class LegacyRolloverProvider
+ extends RolloverSignerSecretProvider {
+ private boolean initCalled;
+
+ @Override
+ public void init(Properties config, ServletContext servletContext,
+ long tokenValidity) throws Exception {
+ this.initCalled = true;
+ super.init(config, servletContext, tokenValidity);
+ }
+
+ @Override
+ protected byte[] generateNewSecret() {
+ return SECRET;
+ }
+ }
+
+ /** A provider written against the servlet-free method. */
+ private static final class ContextProvider extends SignerSecretProvider {
+ private SecretProviderContext seen;
+
+ @Override
+ public void initialize(Properties config, SecretProviderContext context,
+ long tokenValidity) {
+ this.seen = context;
+ }
+
+ @Override
+ public byte[] getCurrentSecret() {
+ return SECRET;
+ }
+
+ @Override
+ public byte[][] getAllSecrets() {
+ return new byte[][]{SECRET};
+ }
+ }
+
+ /** A provider that implements neither method. */
+ private static final class UninitializableProvider extends SignerSecretProvider {
+ @Override
+ public byte[] getCurrentSecret() {
+ return SECRET;
+ }
+
+ @Override
+ public byte[][] getAllSecrets() {
+ return new byte[][]{SECRET};
+ }
+ }
+
+ @Test
+ public void testLegacyProviderStillReceivesTheServletContext()
+ throws Exception {
+ ServletContext servletContext = mock(ServletContext.class);
+ LegacyProvider provider = new LegacyProvider();
+
+ provider.init(new Properties(), servletContext, 1000);
+
+ assertTrue(provider.initCalled, "the legacy override should have run");
+ assertSame(servletContext, provider.seen);
+ }
+
+ @Test
+ public void testLegacyRolloverProviderIsStillRolledOver() throws Exception {
+ LegacyRolloverProvider provider = new LegacyRolloverProvider();
+ try {
+ provider.init(new Properties(), mock(ServletContext.class), 100000);
+
+ // The override ran, and chaining to super still reached
+ // RolloverSignerSecretProvider.initialize, which seeds the secret. Were
+ // the bridge dispatched statically, this would silently be null.
+ assertTrue(provider.initCalled, "the legacy override should have run");
+ assertArrayEquals(SECRET, provider.getCurrentSecret());
+ } finally {
+ provider.destroy();
+ }
+ }
+
+ @Test
+ public void testContextProviderIsReachedThroughTheDeprecatedEntryPoint()
+ throws Exception {
+ Map
+ * The filter reports why through + * {@link javax.servlet.http.HttpServletResponse#sendError}. That detail used + * to arrive in the HTTP reason phrase; since Jetty 12 the phrase is always + * the canonical text for the status code and the detail is in the body, so + * read it from there. An authorization denial arrives instead as the JSON + * envelope {@link HttpExceptionUtils} writes, which + * {@link HttpExceptionUtils#getResponseDetail} leaves untouched - so a + * denial neither matches here nor has its body consumed before + * {@link HttpExceptionUtils#validateResponse} below rebuilds the exception + * from it. + *
+ * Matching {@link #ANONYMOUS_REQUESTS_DISALLOWED} on a substring rather than + * on the whole value is forced by the move into the body: the container + * wraps the reason in an error page. + * + * @param conn a connection whose response status has been read + * @return true when the response names an authentication failure + */ + private static boolean isAuthenticationFailure(HttpURLConnection conn) { + String detail = HttpExceptionUtils.getResponseDetail(conn); + return detail.contains(ANONYMOUS_REQUESTS_DISALLOWED) + || detail.contains(INVALID_SIGNATURE); + } + public static class KMSKeyVersion extends KeyVersion { public KMSKeyVersion(String keyName, String versionName, byte[] material) { super(keyName, versionName, material); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/AdminAuthorizedServlet.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/AdminAuthorizedServlet.java index a4b05a1d534b85..8b17db7558f462 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/AdminAuthorizedServlet.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/AdminAuthorizedServlet.java @@ -23,7 +23,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.servlet.DefaultServlet; +import org.eclipse.jetty.ee8.servlet.DefaultServlet; /** * General servlet which is admin-authorized. diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index d17874d78aca82..3ff58b1b06eb1a 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -28,6 +28,7 @@ import java.net.MalformedURLException; import java.net.URI; import java.net.URL; +import java.nio.channels.ServerSocketChannel; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -89,7 +90,17 @@ import org.apache.hadoop.util.ReflectionUtils; import org.apache.hadoop.util.Shell; import org.apache.hadoop.util.StringUtils; +import org.eclipse.jetty.ee8.nested.SessionHandler; +import org.eclipse.jetty.ee8.servlet.ErrorPageErrorHandler; +import org.eclipse.jetty.ee8.servlet.FilterHolder; +import org.eclipse.jetty.ee8.servlet.FilterMapping; +import org.eclipse.jetty.ee8.servlet.ServletContextHandler; +import org.eclipse.jetty.ee8.servlet.ServletHandler; +import org.eclipse.jetty.ee8.servlet.ServletHolder; +import org.eclipse.jetty.ee8.servlet.ServletMapping; +import org.eclipse.jetty.ee8.webapp.WebAppContext; import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.server.ConnectionFactory; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Handler; @@ -102,21 +113,11 @@ import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.server.handler.ContextHandlerCollection; -import org.eclipse.jetty.server.handler.HandlerCollection; -import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.server.handler.StatisticsHandler; -import org.eclipse.jetty.server.session.SessionHandler; -import org.eclipse.jetty.servlet.FilterHolder; -import org.eclipse.jetty.servlet.FilterMapping; -import org.eclipse.jetty.servlet.ServletContextHandler; -import org.eclipse.jetty.servlet.ServletHandler; -import org.eclipse.jetty.servlet.ServletHolder; -import org.eclipse.jetty.servlet.ServletMapping; import org.eclipse.jetty.util.ArrayUtil; -import org.eclipse.jetty.util.MultiException; +import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; -import org.eclipse.jetty.webapp.WebAppContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -173,6 +174,19 @@ public final class HttpServer2 implements FilterContainer { = "hadoop.http.sni.host.check.enabled"; public static final boolean HTTP_SNI_HOST_CHECK_ENABLED_DEFAULT = false; + /** + * Prefix under which a context init parameter reaches Jetty's DefaultServlet. + *
+ * This is not the package the servlet lives in: ee8 moved the class to
+ * org.eclipse.jetty.ee8.servlet but DefaultServlet#getInitParameter still
+ * reads the 9.4 name. Spelling it the other way costs nothing at startup and
+ * silently drops the setting - dirAllowed then falls back to its default of
+ * true and /static starts listing directories - so the prefix is written
+ * once here rather than at each call site.
+ */
+ private static final String DEFAULT_SERVLET_INIT_PREFIX =
+ "org.eclipse.jetty.servlet.Default.";
+
// The ServletContext attribute where the daemon Configuration
// gets stored.
public static final String CONF_CONTEXT_ATTRIBUTE = "hadoop.conf";
@@ -186,7 +200,7 @@ public final class HttpServer2 implements FilterContainer {
protected final Server webServer;
- private final HandlerCollection handlers;
+ private final Handler.Sequence handlers;
private final List
+ * Jetty 9.4's ServerConnector#close released the accept channel as well as
+ * closing it, so a close/open pair rebound the same port. Jetty 12 closes
+ * the channel but keeps the reference, and ServerConnector#open does nothing
+ * at all while that reference is set - it neither rebinds nor complains, and
+ * getLocalPort stays at -2. Only doStop clears it.
+ *
+ * That is enough for a server that is started, because stopping it clears
+ * the channel on the way down. It is not enough here: openListeners binds
+ * before the Server is started, so a caller that opens listeners, stops, and
+ * opens them again gets a connector that silently never comes back.
+ *
+ * The public open(ServerSocketChannel) overload assigns the channel
+ * unconditionally, so handing it a fresh one restores the 9.4 behaviour
+ * without reaching into Jetty's internals. That is done only after Jetty's
+ * own open() has been given the first go and turned out to be a no-op, so
+ * the ordinary first bind runs the stock path unchanged; the replacement
+ * path does not fire the connector's open callbacks, which is a step up
+ * from the nothing it does today and the reason to keep it to that one
+ * case.
+ */
+ private static final class ReopenableServerConnector extends ServerConnector {
+
+ private ReopenableServerConnector(Server server, int acceptors,
+ int selectors) {
+ super(server, acceptors, selectors);
+ }
+
+ @Override
+ public void open() throws IOException {
+ // Always give Jetty the first go, so a connector that has never been
+ // opened takes the stock path and nothing - the open callbacks it fires
+ // among them - is skipped.
+ super.open();
+ if (isStarted() || getLocalPort() != -2 || isOpen()) {
+ return;
+ }
+ // super.open() returned having bound nothing: the connector is holding
+ // a channel it closed. isOpen() being false is what says the channel is
+ // spent - Jetty only closes it from close() when the connector has
+ // acceptors, and binding a second channel to a port the first one still
+ // holds would fail and leak the first.
+ ServerSocketChannel channel = openAcceptChannel();
+ try {
+ open(channel);
+ } catch (Throwable t) {
+ try {
+ channel.close();
+ } catch (IOException suppressed) {
+ t.addSuppressed(suppressed);
+ }
+ throw t;
+ }
+ }
+ }
+
/**
* Create bind exception by wrapping the bind exception thrown.
* @param listener
@@ -1619,7 +1749,7 @@ void openListeners() throws Exception {
* @throws Exception exception.
*/
public void stop() throws Exception {
- MultiException exception = null;
+ ExceptionUtil.MultiException exception = null;
if (this.configurationChangeMonitor.isPresent()) {
try {
this.configurationChangeMonitor.get().cancel();
@@ -1670,9 +1800,10 @@ public void stop() throws Exception {
}
- private MultiException addMultiException(MultiException exception, Exception e) {
+ private ExceptionUtil.MultiException addMultiException(
+ ExceptionUtil.MultiException exception, Exception e) {
if(exception == null){
- exception = new MultiException();
+ exception = new ExceptionUtil.MultiException();
}
exception.add(e);
return exception;
@@ -1960,9 +2091,7 @@ public void doFilter(ServletRequest request,
*/
private String inferMimeType(ServletRequest request) {
String path = ((HttpServletRequest)request).getRequestURI();
- ServletContextHandler.Context sContext =
- (ServletContextHandler.Context)config.getServletContext();
- String mime = sContext.getMimeType(path);
+ String mime = config.getServletContext().getMimeType(path);
return (mime == null) ? null : mime;
}
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java
index f99476fc4d148d..6cdb3d5b0f0737 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java
@@ -30,6 +30,14 @@
/**
* This class collects all the metrics of Jetty's StatisticsHandler
* and expose them as Hadoop Metrics.
+ *
+ * Jetty 12 rebuilt StatisticsHandler around the core request lifecycle. The
+ * dispatch counters it used to publish are now handle counters measuring the
+ * same thing under a different name, and are read as such here so the metric
+ * names Hadoop emits do not move. Its four async counters and its count of
+ * expired async requests have no counterpart, because the core no longer sees
+ * servlet async activity, so those five metrics are no longer emitted rather
+ * than reported as a constant.
*/
@InterfaceAudience.Private
@InterfaceStability.Unstable
@@ -42,53 +50,33 @@ public class HttpServer2Metrics {
private final int acceptorThreads;
private final int selectorThreads;
- @Metric("number of requested that have been asynchronously dispatched")
- public int asyncDispatches() {
- return handler.getAsyncDispatches();
- }
- @Metric("total number of async requests")
- public int asyncRequests() {
- return handler.getAsyncRequests();
- }
- @Metric("currently waiting async requests")
- public int asyncRequestsWaiting() {
- return handler.getAsyncRequestsWaiting();
- }
- @Metric("maximum number of waiting async requests")
- public int asyncRequestsWaitingMax() {
- return handler.getAsyncRequestsWaitingMax();
- }
@Metric("number of dispatches")
public int dispatched() {
- return handler.getDispatched();
+ return handler.getHandleTotal();
}
@Metric("number of dispatches currently active")
public int dispatchedActive() {
- return handler.getDispatchedActive();
+ return handler.getHandleActive();
}
@Metric("maximum number of active dispatches being handled")
public int dispatchedActiveMax() {
- return handler.getDispatchedActiveMax();
+ return handler.getHandleActiveMax();
}
@Metric("maximum time spend in dispatch handling (in ms)")
public long dispatchedTimeMax() {
- return handler.getDispatchedTimeMax();
+ return handler.getHandleTimeMax();
}
@Metric("mean time spent in dispatch handling (in ms)")
public double dispatchedTimeMean() {
- return handler.getDispatchedTimeMean();
+ return handler.getHandleTimeMean();
}
@Metric("standard deviation for dispatch handling (in ms)")
public double dispatchedTimeStdDev() {
- return handler.getDispatchedTimeStdDev();
+ return handler.getHandleTimeStdDev();
}
@Metric("total time spent in dispatch handling (in ms)")
public long dispatchedTimeTotal() {
- return handler.getDispatchedTimeTotal();
- }
- @Metric("number of async requests requests that have expired")
- public int expires() {
- return handler.getExpires();
+ return handler.getHandleTimeTotal();
}
@Metric("number of requests")
public int requests() {
@@ -140,11 +128,11 @@ public int responses5xx() {
}
@Metric("total number of bytes across all responses")
public long responsesBytesTotal() {
- return handler.getResponsesBytesTotal();
+ return handler.getBytesWritten();
}
@Metric("time in milliseconds stats have been collected for")
public long statsOnMs() {
- return handler.getStatsOnMs();
+ return handler.getStatisticsDuration().toMillis();
}
@Metric("maximum number of threads in the pool")
public int maxThreads() {
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/JettyUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/JettyUtils.java
index 29c0930968ede7..09b08d411725ea 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/JettyUtils.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/JettyUtils.java
@@ -18,6 +18,8 @@
package org.apache.hadoop.http;
+import javax.servlet.http.HttpServletResponse;
+
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
@@ -30,6 +32,26 @@ public final class JettyUtils {
public static final String UTF_8 = "charset=utf-8";
public static final int HEADER_SIZE = 1024 * 64;
+ /**
+ * Clears the content type a response is carrying, charset included.
+ *
+ * Every server built by {@link HttpServer2} runs
+ * {@code QuotingInputFilter}, which sets {@code text/plain; charset=utf-8}
+ * before the request reaches the resource. A JAX-RS resource that picks its
+ * own content type has to undo that first, and {@code setContentType(null)}
+ * alone is not enough on Jetty 12: it drops the charset but remembers that
+ * one had been set explicitly, so the next content type gets that memory
+ * appended to it - literally {@code ;charset=null} for a type that carries
+ * no charset of its own, such as {@code application/octet-stream} or
+ * {@code application/xml}. Clearing the encoding as well resets that state.
+ *
+ * @param response the response to clear
+ */
+ public static void clearContentType(HttpServletResponse response) {
+ response.setContentType(null);
+ response.setCharacterEncoding(null);
+ }
+
private JettyUtils() {
}
}
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/ProfileOutputServlet.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/ProfileOutputServlet.java
index 1ecc21f3753ceb..3c69eca6e6ae2f 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/ProfileOutputServlet.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/ProfileOutputServlet.java
@@ -25,7 +25,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.eclipse.jetty.servlet.DefaultServlet;
+import org.eclipse.jetty.ee8.servlet.DefaultServlet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/WebServlet.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/WebServlet.java
index 2eb6c2beb16a62..765709625e5110 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/WebServlet.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/WebServlet.java
@@ -22,7 +22,7 @@
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.eclipse.jetty.servlet.DefaultServlet;
+import org.eclipse.jetty.ee8.servlet.DefaultServlet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java
index 7363ca0ba64505..7c5da6fbd268ff 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java
@@ -36,8 +36,8 @@
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.util.HttpExceptionUtils;
-import org.eclipse.jetty.server.Response;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -272,11 +272,21 @@ public void proceed() throws IOException, ServletException {
@Override
public void sendError(int code, String message) throws IOException {
- if (httpResponse instanceof Response) {
- ((Response)httpResponse).setStatusWithReason(code, message);
- }
-
- httpResponse.sendError(code, message);
+ // Reported as the JSON envelope HttpExceptionUtils writes, rather than
+ // handed to sendError for the container to render.
+ //
+ // The message used to travel in the reason phrase. Jetty 12 does not
+ // send one, and what sendError leaves in its place is the container's
+ // HTML error page - which WebHdfsFileSystem refuses on its content type
+ // and reports as a bare "Bad Request", losing the one thing the caller
+ // needed to know. The envelope is what the rest of Hadoop's HTTP
+ // surface already answers refusals with, and what its clients parse.
+ //
+ // Only the servlet side moves. The two Netty implementations of
+ // HttpInteraction that serve the DataNode build their own responses and
+ // never went through a reason phrase.
+ HttpExceptionUtils.createServletExceptionResponse(httpResponse, code,
+ new IOException(message));
}
}
}
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/web/DelegationTokenAuthenticationHandler.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/web/DelegationTokenAuthenticationHandler.java
index f4ede6f35edb0c..29e311739e18f4 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/web/DelegationTokenAuthenticationHandler.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/web/DelegationTokenAuthenticationHandler.java
@@ -302,7 +302,15 @@ public boolean managementOperation(AuthenticationToken token,
requestUgi.getShortUserName());
map = Collections.singletonMap("long", expirationTime);
} catch (IOException ex) {
- throw new AuthenticationException(ex.toString(), ex);
+ // Reported the same way as the authorization failure above,
+ // rather than thrown for AuthenticationFilter to turn into a
+ // bare status. Letting it out used to be survivable because
+ // the reason phrase carried the detail; Jetty 12 does not
+ // send one, and the container's error page is HTML, which is
+ // not what the client on the other end parses.
+ HttpExceptionUtils.createServletExceptionResponse(response,
+ HttpServletResponse.SC_FORBIDDEN, ex);
+ requestContinues = false;
}
}
break;
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HttpExceptionUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HttpExceptionUtils.java
index 43441a5560a338..b4e4fd6de30060 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HttpExceptionUtils.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HttpExceptionUtils.java
@@ -23,13 +23,17 @@
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
+import java.io.BufferedInputStream;
+import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InputStreamReader;
import java.io.Writer;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.net.HttpURLConnection;
+import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
@@ -134,6 +138,11 @@ private static
* NOTE: this method will throw the deserialized exception even if not
* declared in the
+ * When the response does not carry the JSON envelope - a container error
+ * page, say - the detail is taken from the body via
+ * {@link #getResponseDetail}, because that is where a servlet's reason now
+ * is: Jetty 12 no longer puts one in the HTTP reason phrase.
*
* @param conn the
+ * A servlet reports its reason through
+ * {@link HttpServletResponse#sendError}, and that detail used to reach the
+ * caller in the reason phrase, which
+ * {@link HttpURLConnection#getResponseMessage()} returns. Jetty 12 never
+ * puts a reason phrase on the wire: the phrase is now always the canonical
+ * text for the status code - "Forbidden", "Gone" - and the detail is in the
+ * body instead. Read the body, and fall back to the phrase when there is
+ * none.
+ *
+ * For a response that carries the JSON envelope this class writes, prefer
+ * {@link #validateResponse}, which rebuilds the original exception. This is
+ * for everything else: a container's error page, or a plain-text reason.
+ * A JSON body is therefore left alone here and the phrase reported instead:
+ * the envelope is sent with {@code setStatus} rather than sendError, so its
+ * phrase was the canonical text for the status code before Jetty 12 and
+ * still is, and quoting the envelope back as free text would replace a
+ * readable "Forbidden" with a line of JSON.
+ *
+ * @param conn a connection whose response status has been read
+ * @return a description of the failure, never null
+ */
+ public static String getResponseDetail(HttpURLConnection conn) {
+ String body = "";
+ if (!isJson(conn.getContentType())) {
+ try (InputStream es = conn.getErrorStream()) {
+ if (es != null) {
+ body = toPlainText(readCapped(es));
+ }
+ } catch (IOException ex) {
+ // nothing to add: fall through to the reason phrase
+ }
+ }
+ if (!body.isEmpty()) {
+ return body;
+ }
+ return responsePhrase(conn);
+ }
+
+ /**
+ * Describes a failure whose body has already been read - and failed - as the
+ * JSON envelope. The body is the only place a servlet's reason can be now,
+ * so rewind and read it as text. The envelope guard {@link
+ * #getResponseDetail} applies does not belong here: nothing that parsed as
+ * the envelope reaches this point, so there is no envelope to protect.
+ *
+ * @param es the buffered error stream, marked at its start, or null
+ * @param conn the connection it came from
+ * @return a description of the failure, never null
+ */
+ private static String rewoundDetail(InputStream es, HttpURLConnection conn) {
+ if (es != null) {
+ try {
+ es.reset();
+ String body = toPlainText(readCapped(es));
+ if (!body.isEmpty()) {
+ return body;
+ }
+ } catch (IOException ex) {
+ // read too far to rewind: fall through to the reason phrase
+ }
+ }
+ return responsePhrase(conn);
+ }
+
+ /**
+ * The HTTP reason phrase, or "" when there is none. Since Jetty 12 this is
+ * always the canonical text for the status code.
+ */
+ private static String responsePhrase(HttpURLConnection conn) {
+ try {
+ String phrase = conn.getResponseMessage();
+ return phrase == null ? "" : phrase;
+ } catch (IOException ex) {
+ return "";
+ }
+ }
+
+ /**
+ * Hides {@link InputStream#close()} from a reader that would otherwise close
+ * the stream on its way out. The JSON reader closes its source even when the
+ * parse failed, and a closed stream can no longer be rewound and read as
+ * text. The caller keeps ownership and closes the real stream itself.
+ */
+ private static InputStream shielded(InputStream in) {
+ if (in == null) {
+ return null;
+ }
+ return new FilterInputStream(in) {
+ @Override
+ public void close() {
+ // the caller owns the stream
+ }
+ };
+ }
+
+ /**
+ * Whether the content type names the JSON error envelope. The header can
+ * carry parameters - "application/json; charset=utf-8" - so this matches a
+ * prefix rather than the whole value.
+ */
+ private static boolean isJson(String contentType) {
+ return contentType != null
+ && contentType.trim().toLowerCase().startsWith(APPLICATION_JSON_MIME);
+ }
+
+ private static String readCapped(InputStream in) throws IOException {
+ InputStreamReader reader =
+ new InputStreamReader(in, StandardCharsets.UTF_8);
+ StringBuilder sb = new StringBuilder();
+ char[] buf = new char[1024];
+ int n;
+ while (sb.length() < MAX_RESPONSE_DETAIL_CHARS
+ && (n = reader.read(buf)) != -1) {
+ sb.append(buf, 0, Math.min(n, MAX_RESPONSE_DETAIL_CHARS - sb.length()));
+ }
+ return sb.toString();
+ }
+
+ /**
+ * Reduces a response body to something readable in a log line. A container
+ * that renders sendError as an HTML page buries the message in markup; strip
+ * it out rather than quoting the page.
+ */
+ private static String toPlainText(String body) {
+ String text = body;
+ if (text.indexOf('<') >= 0) {
+ text = text.replaceAll("(?s)<(script|style)\\b.*?", " ")
+ .replaceAll("(?s)<[^>]*>", " ");
+ }
+ text = text.replace("<", "<").replace(">", ">")
+ .replace(""", "\"").replace("'", "'")
+ .replace("&", "&");
+ return text.replaceAll("\\s+", " ").trim();
+ }
+
}
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
index be1baa97c88109..a1b67a697078bb 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
@@ -36,7 +36,8 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.eclipse.jetty.util.log.Log;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
*
@@ -58,6 +59,9 @@
*
+ * Absence of an annotation is not a licence to leak. Several of these are
+ * the oldest classes in hadoop-auth and carry no audience marker at all,
+ * which does not make them less public in practice; only an explicit
+ * Private marker excuses a class, and only two here have one -
+ * AuthenticationFilter, and HttpServer2, which names Jetty deliberately
+ * (addHandlerAtFront takes a Handler) and is documented as internal.
+ */
+ @Test
+ public void testPublicApiNeverNamesJetty() {
+ List
+ * Jetty 12 remembers that a charset had been set explicitly even after the
+ * content type carrying it is cleared, and appends that memory to the next
+ * content type - as ";charset=null" for a type with no charset of its own.
+ * Only types that assume a charset, application/json among them, escape it,
+ * which is why octet-stream and xml are the ones asserted here.
+ */
+ @Test
+ public void testClearedContentTypeCarriesNoCharset() throws Exception {
+ for (String type : new String[] {MediaType.APPLICATION_OCTET_STREAM,
+ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON}) {
+ URL url = new URL(baseUrl, "/owncontenttype?type=" + type);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+ conn.connect();
+ assertEquals(HttpServletResponse.SC_OK, conn.getResponseCode());
+ assertEquals(type, conn.getContentType(),
+ "the cleared charset came back for " + type);
+ conn.disconnect();
+ }
+ }
+
+ /**
+ * /static must never list what is in it. The setting that stops it is a
+ * context init parameter, which the DefaultServlet reads under a prefix of
+ * its own choosing: get the prefix wrong and the parameter is not rejected,
+ * it is ignored, and dirAllowed falls back to its default of true. That is
+ * silent, so the endpoint is asserted rather than the setting.
+ */
+ @Test
+ public void testStaticContextDoesNotListDirectories() throws Exception {
+ URL staticUrl = new URL(baseUrl, "/static/");
+ HttpURLConnection conn = (HttpURLConnection) staticUrl.openConnection();
+ conn.connect();
+ assertEquals(HttpServletResponse.SC_FORBIDDEN, conn.getResponseCode(),
+ "/static served a directory listing");
+
+ // The context is otherwise working, so the 403 above is dirAllowed doing
+ // its job and not the whole context being broken.
+ URL cssUrl = new URL(baseUrl, "/static/test.css");
+ conn = (HttpURLConnection) cssUrl.openConnection();
+ conn.connect();
+ assertEquals(HttpServletResponse.SC_OK, conn.getResponseCode());
+ }
+
+ /**
+ * A path with an empty segment has to reach the servlet, which is where
+ * Hadoop decides what it means. Jetty 12 rejects one at the connector by
+ * default, with a bare 400 and no body - WebHDFS clients parse a JSON
+ * RemoteException out of that response, and there is nothing there to parse.
+ * The ambiguities that actually matter stay rejected, so an encoded
+ * separator is asserted here too.
+ */
+ @Test
+ public void testHadoopPathsReachTheServlet() throws Exception {
+ assertPathInfo("//tmp//file", "/pathinfo//tmp//file");
+ // A file whose name contains a '%' arrives as %25.
+ assertPathInfo("/a%b", "/pathinfo/a%25b");
+ assertPathInfo("/@;%$", "/pathinfo/%40%3B%25%24");
+ // A '\' is a path separator on Windows and just a character on HDFS.
+ assertPathInfo("/a\\b", "/pathinfo/a%5Cb");
+ }
+
+ /**
+ * The other half of the same setting: a path that would read as one thing
+ * to a filter and another to a servlet still has to be refused.
+ */
+ @Test
+ public void testAmbiguousPathsAreStillRejected() throws Exception {
+ assertNotServed("an encoded path separator", "/pathinfo/tmp%2Ffile");
+ assertNotServed("an encoded dot-segment", "/pathinfo/a%2E%2E%2Fb");
+ }
+
+ private static void assertPathInfo(String expected, String path)
+ throws Exception {
+ URL url = new URL(baseUrl, path);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+ conn.connect();
+ assertEquals(HttpServletResponse.SC_OK, conn.getResponseCode(),
+ path + " was rejected before the servlet ran");
+ assertEquals(expected, readOutput(url).trim(),
+ path + " did not reach the servlet as sent");
+ }
+
+ private static void assertNotServed(String what, String path)
+ throws Exception {
+ URL url = new URL(baseUrl, path);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+ conn.connect();
+ assertThat(conn.getResponseCode())
+ .as(what + " must not be served")
+ .isNotEqualTo(HttpServletResponse.SC_OK);
+ }
+
@Test
public void testHttpServer2Metrics() throws Exception {
final HttpServer2Metrics metrics = server.getMetrics();
@@ -286,8 +418,10 @@ public void testHttpServer2Metrics() throws Exception {
(HttpURLConnection)servletUrl.openConnection();
conn.connect();
assertThat(conn.getResponseCode()).isEqualTo(200);
- final int after = metrics.responses2xx();
- assertThat(after).isGreaterThan(before);
+ // Jetty 12 books the response when the exchange completes on the server,
+ // which can be after the client has read the status line, so the counter
+ // is given a moment rather than read straight away.
+ GenericTestUtils.waitFor(() -> metrics.responses2xx() > before, 50, 10000);
}
@Test
@@ -323,9 +457,12 @@ public void testHttpServer2ThreadPoolMetrics() throws Exception {
}
/**
- * Jetty StatisticsHandler must be inserted via Server#insertHandler
- * instead of Server#setHandler. The server fails to start if
- * the handler is added by setHandler.
+ * Jetty StatisticsHandler must be inserted via Server#insertHandler instead
+ * of Server#setHandler. On 9.4 the difference showed up as a server that
+ * refused to start, so the test could assert the failure; Jetty 12 starts a
+ * childless handler quite happily and serves 404s from it, which is worse.
+ * So the assertion is that the server still serves after the handler goes
+ * in - the reason to prefer insertHandler in the first place.
*/
@Test
public void testSetStatisticsHandler() throws Exception {
@@ -334,11 +471,30 @@ public void testSetStatisticsHandler() throws Exception {
conf.setBoolean(
CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, false);
final HttpServer2 testServer = createTestServer(conf);
- testServer.webServer.setHandler(new StatisticsHandler());
+ testServer.addServlet("echo", "/echo", EchoServlet.class);
+
+ final Handler tree = testServer.webServer.getHandler();
+ assertThat(tree).isNotNull();
+ final StatisticsHandler statistics = new StatisticsHandler();
+ testServer.webServer.insertHandler(statistics);
+ assertThat(statistics.getHandler())
+ .as("insertHandler keeps the handler tree underneath")
+ .isSameAs(tree);
+
try {
testServer.start();
- fail("IOException should be thrown.");
- } catch (IOException ignore) {
+ final URL echoUrl = new URL(getServerURL(testServer), "/echo?a=b");
+ final HttpURLConnection conn =
+ (HttpURLConnection) echoUrl.openConnection();
+ conn.connect();
+ assertThat(conn.getResponseCode())
+ .as("the webapp stopped serving once StatisticsHandler was inserted")
+ .isEqualTo(HttpServletResponse.SC_OK);
+ // Booked when the exchange completes on the server, which can be after
+ // the client has read the status line - see testHttpServer2Metrics.
+ GenericTestUtils.waitFor(() -> statistics.getRequests() > 0, 50, 10000);
+ } finally {
+ testServer.stop();
}
}
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java
index f39dd1a103b452..a5e30fe0ef437c 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java
@@ -18,10 +18,13 @@
package org.apache.hadoop.security.http;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.verify;
import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
@@ -67,7 +70,8 @@ public void testNoHeaderDefaultConfigBadRequest()
thenReturn(BROWSER_AGENT);
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -75,8 +79,7 @@ public void testNoHeaderDefaultConfigBadRequest()
filter.init(filterConfig);
filter.doFilter(mockReq, mockRes, mockChain);
- verify(mockRes, atLeastOnce()).sendError(
- HttpServletResponse.SC_BAD_REQUEST, EXPECTED_MESSAGE);
+ assertRejected(mockRes, writer);
verifyZeroInteractions(mockChain);
}
@@ -102,7 +105,8 @@ public void testNoHeaderCustomAgentConfigBadRequest()
thenReturn("curl");
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -110,8 +114,7 @@ public void testNoHeaderCustomAgentConfigBadRequest()
filter.init(filterConfig);
filter.doFilter(mockReq, mockRes, mockChain);
- verify(mockRes, atLeastOnce()).sendError(
- HttpServletResponse.SC_BAD_REQUEST, EXPECTED_MESSAGE);
+ assertRejected(mockRes, writer);
verifyZeroInteractions(mockChain);
}
@@ -134,7 +137,8 @@ public void testNoHeaderDefaultConfigNonBrowserGoodRequest()
thenReturn(NON_BROWSER);
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -162,7 +166,8 @@ public void testHeaderPresentDefaultConfigGoodRequest()
thenReturn("valueUnimportant");
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -191,7 +196,8 @@ public void testHeaderPresentCustomHeaderConfigGoodRequest()
thenReturn("valueUnimportant");
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -222,7 +228,8 @@ public void testMissingHeaderWithCustomHeaderConfigBadRequest()
thenReturn(null);
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -254,7 +261,8 @@ public void testMissingHeaderNoMethodsToIgnoreConfigBadRequest()
thenReturn("GET");
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -286,7 +294,8 @@ public void testMissingHeaderIgnoreGETMethodConfigGoodRequest()
thenReturn("GET");
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -318,7 +327,8 @@ public void testMissingHeaderMultipleIgnoreMethodsConfigGoodRequest()
thenReturn("OPTIONS");
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -350,7 +360,8 @@ public void testMissingHeaderMultipleIgnoreMethodsConfigBadRequest()
thenReturn("PUT");
// Objects to verify interactions based on request
- HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ StringWriter writer = new StringWriter();
+ HttpServletResponse mockRes = mockResponse(writer);
FilterChain mockChain = Mockito.mock(FilterChain.class);
// Object under test
@@ -360,4 +371,34 @@ public void testMissingHeaderMultipleIgnoreMethodsConfigBadRequest()
verifyZeroInteractions(mockChain);
}
+
+ /**
+ * A refusal is reported as the JSON envelope HttpExceptionUtils writes, not
+ * handed to sendError, so what is asserted is the status and the message the
+ * caller will actually be able to read.
+ *
+ * @param res the mocked response the filter wrote to
+ * @param body what the filter wrote into it
+ */
+ private static void assertRejected(HttpServletResponse res, StringWriter body) {
+ verify(res, atLeastOnce()).setStatus(HttpServletResponse.SC_BAD_REQUEST);
+ assertTrue(body.toString().contains(EXPECTED_MESSAGE),
+ "refusal did not carry the reason: " + body);
+ }
+
+ /**
+ * A mock response the filter can write a refusal into. Every caller gets a
+ * writer whether or not it looks at one, because the filter now reports a
+ * refusal by writing the error rather than by calling sendError.
+ *
+ * @param body collects whatever the filter writes
+ * @return the mock response
+ * @throws IOException never, but getWriter declares it
+ */
+ private static HttpServletResponse mockResponse(StringWriter body)
+ throws IOException {
+ HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
+ Mockito.when(res.getWriter()).thenReturn(new PrintWriter(body));
+ return res;
+ }
}
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/web/TestWebDelegationToken.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/web/TestWebDelegationToken.java
index 25756dd4277944..ce3867afd483b1 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/web/TestWebDelegationToken.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/web/TestWebDelegationToken.java
@@ -21,6 +21,7 @@
import org.apache.commons.io.IOUtils;
import org.apache.hadoop.io.Text;
import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.hadoop.security.AccessControlException;
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.security.authentication.KerberosTestUtils;
import org.apache.hadoop.security.authentication.client.AuthenticationException;
@@ -35,12 +36,12 @@
import org.apache.hadoop.test.GenericTestUtils;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
-import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.ee8.servlet.ServletContextHandler;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.eclipse.jetty.servlet.FilterHolder;
-import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jetty.ee8.servlet.FilterHolder;
+import org.eclipse.jetty.ee8.servlet.ServletHolder;
import org.slf4j.event.Level;
import javax.security.auth.Subject;
@@ -78,6 +79,7 @@
import java.util.concurrent.Callable;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -344,6 +346,28 @@ public void testDelegationTokenAuthenticatorCallsWithQueryString()
testDelegationTokenAuthenticatorCalls(true);
}
+ /**
+ * A renewal refused by the server has to reach the caller as the exception
+ * the server raised, carrying the reason it gave.
+ *
+ * The handler reports this through HttpExceptionUtils, which writes the
+ * error as JSON and lets the client rebuild the original type. These calls
+ * used to assert that the message contained "403" instead, which only held
+ * while the body was unparseable: the client fell back to a generic
+ * IOException that quoted the status line. That fallback is what the
+ * assertion was really pinning, so it went on passing while the detail the
+ * caller needed was being thrown away.
+ *
+ * @param ex the exception the client raised
+ */
+ private static void assertRenewRefused(Exception ex) {
+ assertInstanceOf(AccessControlException.class, ex,
+ "renewal refusal did not survive the round trip as the server's own"
+ + " exception; got " + ex.getClass().getName() + ": "
+ + ex.getMessage());
+ assertTrue(ex.getMessage().contains("renew"),
+ "refusal lost the server's reason: " + ex.getMessage());
+ }
private void testDelegationTokenAuthenticatorCalls(final boolean useQS)
throws Exception {
@@ -394,7 +418,7 @@ private void testDelegationTokenAuthenticatorCalls(final boolean useQS)
aUrl.renewDelegationToken(authURL2, token);
fail();
} catch (Exception ex) {
- assertTrue(ex.getMessage().contains("403"));
+ assertRenewRefused(ex);
}
aUrl.getDelegationToken(authURL, token, FOO_USER);
@@ -817,7 +841,7 @@ public Void call() throws Exception {
aUrl.renewDelegationToken(url, token, doAsUser);
fail();
} catch (Exception ex) {
- assertTrue(ex.getMessage().contains("403"));
+ assertRenewRefused(ex);
}
aUrl.getDelegationToken(url, token, FOO_USER, doAsUser);
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHttpExceptionUtils.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHttpExceptionUtils.java
index f3f0e43b394194..d501866fc106a3 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHttpExceptionUtils.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHttpExceptionUtils.java
@@ -37,6 +37,8 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@@ -109,12 +111,83 @@ public void testValidateResponseNonJsonErrorMessage() throws Exception {
when(conn.getErrorStream()).thenReturn(is);
when(conn.getResponseMessage()).thenReturn("msg");
when(conn.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
+ // The body wins over the reason phrase: a servlet's reason travels in the
+ // body now, so "stream" is the detail and "msg" is only the canonical
+ // text for the status code.
LambdaTestUtils.interceptAndValidateMessageContains(IOException.class,
- Arrays.asList(Integer.toString(HttpURLConnection.HTTP_BAD_REQUEST), "msg",
+ Arrays.asList(Integer.toString(HttpURLConnection.HTTP_BAD_REQUEST), "stream",
"com.fasterxml.jackson.core.JsonParseException"),
() -> HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED));
}
+ @Test
+ public void testValidateResponseHtmlErrorPageReportsTheReason()
+ throws Exception {
+ // What AuthenticationFilter's sendError looks like on the wire.
+ String page = " Reason: Invalid signaturethrows of the method signature.
+ * HttpURLConnection.
* @param expectedStatus the expected HTTP status code.
@@ -147,8 +156,12 @@ public static void validateResponse(HttpURLConnection conn,
Exception toThrow;
InputStream es = null;
try {
- es = conn.getErrorStream();
- Map json = JsonSerialization.mapReader().readValue(es);
+ InputStream raw = conn.getErrorStream();
+ if (raw != null) {
+ es = new BufferedInputStream(raw);
+ es.mark(ERROR_BODY_REWIND_LIMIT);
+ }
+ Map json = JsonSerialization.mapReader().readValue(shielded(es));
json = (Map) json.get(ERROR_JSON);
String exClass = (String) json.get(ERROR_CLASSNAME_JSON);
String exMsg = (String) json.get(ERROR_MESSAGE_JSON);
@@ -175,7 +188,7 @@ public static void validateResponse(HttpURLConnection conn,
} catch (Exception ex) {
toThrow = new IOException(String.format(
"HTTP status [%d], message [%s], URL [%s], exception [%s]",
- conn.getResponseCode(), conn.getResponseMessage(), conn.getURL(),
+ conn.getResponseCode(), rewoundDetail(es, conn), conn.getURL(),
ex.toString()), ex);
} finally {
if (es != null) {
@@ -190,4 +203,157 @@ public static void validateResponse(HttpURLConnection conn,
}
}
+ /** How much of a failed response body is worth quoting back. */
+ private static final int MAX_RESPONSE_DETAIL_CHARS = 4096;
+
+ /**
+ * How much of an error body {@link #validateResponse} keeps buffered so it
+ * can be rewound and read as text once the JSON parse has failed. Sized to
+ * hold {@link #MAX_RESPONSE_DETAIL_CHARS} characters of any UTF-8 body. A
+ * body longer than this still parses - the reader runs straight through it -
+ * it just cannot be rewound, which leaves the reason phrase as the fallback,
+ * as it was before.
+ */
+ private static final int ERROR_BODY_REWIND_LIMIT =
+ 4 * MAX_RESPONSE_DETAIL_CHARS;
+
+ /**
+ * Describes why a request failed, preferring the response body over the HTTP
+ * reason phrase.
+ * HTTP ERROR 403
HTTP ERROR 403
\n"
+ + "
\n"
+ + "\n\n";
+ String detail = HttpExceptionUtils.getResponseDetail(
+ connectionReturning(page, "Forbidden"));
+ assertTrue(detail.contains("the real reason"), detail);
+ assertFalse(detail.contains("<"), "markup survived: " + detail);
+ }
+
+ @Test
+ public void testResponseDetailFallsBackToThePhrase() throws Exception {
+ assertEquals("Forbidden", HttpExceptionUtils.getResponseDetail(
+ connectionReturning(null, "Forbidden")));
+ assertEquals("Forbidden", HttpExceptionUtils.getResponseDetail(
+ connectionReturning(" \n ", "Forbidden")));
+ }
+
+ @Test
+ public void testResponseDetailIsNeverNull() throws Exception {
+ assertEquals("", HttpExceptionUtils.getResponseDetail(
+ connectionReturning(null, null)));
+ }
+
+ /**
+ * A refusal carrying the JSON envelope is reported by its reason phrase, not
+ * by the envelope. Such a response is sent with setStatus rather than
+ * sendError - see {@link HttpExceptionUtils#createServletExceptionResponse}
+ * - so it never had a reason of its own in the phrase, and quoting the JSON
+ * back would replace a readable "Forbidden" with a line of markup. Callers
+ * that want what is inside it use validateResponse.
+ */
+ @Test
+ public void testResponseDetailLeavesTheJsonEnvelopeAlone() throws Exception {
+ String envelope = "{\"RemoteException\":{\"message\":\"User: client is not"
+ + " allowed to impersonate foo1\",\"exception\":\"AuthorizationException\","
+ + "\"javaClassName\":\"org.apache.hadoop.security.authorize."
+ + "AuthorizationException\"}}";
+
+ assertEquals("Forbidden", HttpExceptionUtils.getResponseDetail(
+ connectionReturning(envelope, "Forbidden", "application/json")));
+ // the header may carry parameters
+ assertEquals("Forbidden", HttpExceptionUtils.getResponseDetail(
+ connectionReturning(envelope, "Forbidden",
+ "application/json; charset=utf-8")));
+ }
+
+ /**
+ * A body that is not JSON is still preferred, which is the case the reader
+ * exists for: Jetty 12 puts what sendError was given in the body and leaves
+ * the phrase canonical.
+ */
+ @Test
+ public void testResponseDetailStillPrefersANonJsonBody() throws Exception {
+ assertEquals("the real reason", HttpExceptionUtils.getResponseDetail(
+ connectionReturning("the real reason", "Forbidden", "text/plain")));
+ }
}
diff --git a/hadoop-common-project/hadoop-kms/pom.xml b/hadoop-common-project/hadoop-kms/pom.xml
index e2874eb3a8033a..42c3654367ab6d 100644
--- a/hadoop-common-project/hadoop-kms/pom.xml
+++ b/hadoop-common-project/hadoop-kms/pom.xml
@@ -69,8 +69,8 @@
MESSAGE: the real reason
+ * The refusal is thrown from managementOperation rather than from
+ * authenticate because authenticate is reached only by a request that
+ * carries no valid token, and such a request is answered 401, not 403. An
+ * AuthenticationException out of managementOperation is what makes
+ * AuthenticationFilter answer 403 - with the reason in the body only.
+ */
+ public static class FailWhenArmedAuthenticationHandler
+ extends PseudoDelegationTokenAuthenticationHandler {
+
+ private static final AtomicBoolean ARMED = new AtomicBoolean();
+ private static final AtomicInteger REFUSALS = new AtomicInteger();
+
+ @Override
+ public boolean managementOperation(AuthenticationToken token,
+ HttpServletRequest request, HttpServletResponse response)
+ throws IOException, AuthenticationException {
+ if (ARMED.compareAndSet(true, false)) {
+ REFUSALS.incrementAndGet();
+ throw new AuthenticationException(
+ new SignerException("Invalid signature"));
+ }
+ return super.managementOperation(token, request, response);
+ }
+ }
+
+ @Test
+ public void testKMSAuthFailureRetryOn403() throws Exception {
+ // The reason for a refusal reaches the client in the response body, not in
+ // the HTTP reason phrase: Jetty 12 never puts a phrase on the wire. A
+ // client that reads only the phrase sees "Forbidden", fails to recognise
+ // an authentication failure, and gives up without re-authenticating.
+ Configuration conf = new Configuration();
+ File confDir = getTestDir();
+ conf = createBaseKMSConf(confDir, conf);
+ conf.set("hadoop.kms.authentication.type",
+ FailWhenArmedAuthenticationHandler.class.getName());
+ conf.set(KeyAuthorizationKeyProvider.KEY_ACL + "k1.ALL", "*");
+ conf.set(KeyAuthorizationKeyProvider.KEY_ACL + "k2.ALL", "*");
+ writeConf(confDir, conf);
+ FailWhenArmedAuthenticationHandler.ARMED.set(false);
+ FailWhenArmedAuthenticationHandler.REFUSALS.set(0);
+
+ runServer(null, null, confDir, new KMSCallable
+ * The rendering happens before the response is touched. Writing it straight
+ * to the response stream commits the response as soon as that stream is
+ * closed, and once the status line is on the wire a later failure cannot be
+ * reported at all: sendError has nothing left to set, so a topology that
+ * failed half way through goes out as 200 OK with a truncated body.
+ *
+ * @param response the response to send on
+ * @param leaves the nodes to render
+ * @param format the response format, from {@link #parseAcceptHeader}
+ * @throws IOException if the topology could not be rendered or sent
+ */
+ protected void sendTopology(HttpServletResponse response, List
+ * Rendering straight to the response stream committed the response before
+ * the failure was known - sendError then had nothing left to set, and a
+ * truncated topology was answered 200 OK. The response must stay untouched
+ * until the whole dump is in hand.
+ */
+ @Test
+ public void testFailedDumpIsNotAnsweredAsSuccess() throws Exception {
+ NetworkTopologyServlet servlet = new NetworkTopologyServlet() {
+ @Override
+ protected void printTopology(PrintStream stream, List
This class handles job end notification. Submitters of jobs can choose to
@@ -48,6 +49,9 @@
* (eg. SUCCEEDED/KILLED/FAILED)
*/
public class JobEndNotifier implements Configurable {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(JobEndNotifier.class);
+
private static final String JOB_ID = "$jobId";
private static final String JOB_STATUS = "$jobStatus";
@@ -109,11 +113,11 @@ public void setConf(Configuration conf) {
int port = Integer.parseInt(portConf);
proxyToUse = new Proxy(proxyType,
new InetSocketAddress(hostname, port));
- Log.getLog().info("Job end notification using proxy type \""
+ LOG.info("Job end notification using proxy type \""
+ proxyType + "\" hostname \"" + hostname + "\" and port \"" + port
+ "\"");
} catch(NumberFormatException nfe) {
- Log.getLog().warn("Job end notification couldn't parse configured"
+ LOG.warn("Job end notification couldn't parse configured"
+ "proxy's port " + portConf + ". Not going to use a proxy");
}
}
@@ -141,24 +145,24 @@ protected boolean notifyURLOnce() {
private boolean notifyViaBuiltInNotifier() {
boolean success = false;
try {
- Log.getLog().info("Job end notification trying " + urlToNotify);
+ LOG.info("Job end notification trying " + urlToNotify);
HttpURLConnection conn =
(HttpURLConnection) urlToNotify.openConnection(proxyToUse);
conn.setConnectTimeout(timeout);
conn.setReadTimeout(timeout);
conn.setAllowUserInteraction(false);
if(conn.getResponseCode() != HttpURLConnection.HTTP_OK) {
- Log.getLog().warn("Job end notification to " + urlToNotify
+ LOG.warn("Job end notification to " + urlToNotify
+ " failed with code: " + conn.getResponseCode() + " and message \""
+ conn.getResponseMessage() + "\"");
}
else {
success = true;
- Log.getLog().info("Job end notification to " + urlToNotify
+ LOG.info("Job end notification to " + urlToNotify
+ " succeeded");
}
} catch(IOException ioe) {
- Log.getLog().warn("Job end notification to " + urlToNotify + " failed",
+ LOG.warn("Job end notification to " + urlToNotify + " failed",
ioe);
}
return success;
@@ -169,7 +173,7 @@ private boolean notifyViaBuiltInNotifier() {
*/
private boolean notifyViaCustomNotifier() {
try {
- Log.getLog().info("Will be using " + customJobEndNotifierClassName
+ LOG.info("Will be using " + customJobEndNotifierClassName
+ " for Job end notification");
final Class customJobEndNotifierClass =
@@ -180,15 +184,15 @@ private boolean notifyViaCustomNotifier() {
boolean success = customJobEndNotifier.notifyOnce(urlToNotify, conf);
if (success) {
- Log.getLog().info("Job end notification to " + urlToNotify
+ LOG.info("Job end notification to " + urlToNotify
+ " succeeded");
} else {
- Log.getLog().warn("Job end notification to " + urlToNotify
+ LOG.warn("Job end notification to " + urlToNotify
+ " failed");
}
return success;
} catch (Exception e) {
- Log.getLog().warn("Job end notification to " + urlToNotify
+ LOG.warn("Job end notification to " + urlToNotify
+ " failed", e);
return false;
}
@@ -215,24 +219,24 @@ public void notify(JobReport jobReport)
try {
urlToNotify = new URL(userUrl);
} catch (MalformedURLException mue) {
- Log.getLog().warn("Job end notification couldn't parse " + userUrl, mue);
+ LOG.warn("Job end notification couldn't parse " + userUrl, mue);
return;
}
// Send notification
boolean success = false;
while (numTries-- > 0 && !success) {
- Log.getLog().info("Job end notification attempts left " + numTries);
+ LOG.info("Job end notification attempts left " + numTries);
success = notifyURLOnce();
if (!success) {
Thread.sleep(waitInterval);
}
}
if (!success) {
- Log.getLog().warn("Job end notification failed to notify : "
+ LOG.warn("Job end notification failed to notify : "
+ urlToNotify);
} else {
- Log.getLog().info("Job end notification succeeded for "
+ LOG.info("Job end notification succeeded for "
+ jobReport.getJobId());
}
}
diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/webapp/AMWebServices.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/webapp/AMWebServices.java
index 2aa3dcb6441eff..549726a4537260 100644
--- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/webapp/AMWebServices.java
+++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/webapp/AMWebServices.java
@@ -115,8 +115,7 @@ Boolean hasAccess(Job job, HttpServletRequest request) {
}
private void init() {
- //clear content type
- response.setContentType(null);
+ JettyUtils.clearContentType(response);
}
public static Job getJobFromContainerIdString(String cid, AppContext appCtx)
diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/webapp/HsWebServices.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/webapp/HsWebServices.java
index 3efa09ec0d2e51..fcb70552f01fd3 100644
--- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/webapp/HsWebServices.java
+++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/webapp/HsWebServices.java
@@ -150,8 +150,7 @@ private boolean isMRJobContainer(String containerIdStr) {
}
private void init() {
- //clear content type
- response.setContentType(null);
+ JettyUtils.clearContentType(response);
}
@VisibleForTesting
diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/NotificationTestCase.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/NotificationTestCase.java
index 26feba37a0672f..5e044c8003bd1a 100644
--- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/NotificationTestCase.java
+++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/NotificationTestCase.java
@@ -20,8 +20,8 @@
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jetty.ee8.servlet.ServletContextHandler;
+import org.eclipse.jetty.ee8.servlet.ServletHolder;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.io.Text;
diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleChannelHandler.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleChannelHandler.java
index d0d0e74c9970b0..41f3e906b104e2 100644
--- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleChannelHandler.java
+++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleChannelHandler.java
@@ -61,7 +61,6 @@
import org.apache.hadoop.io.SecureIOUtils;
import org.apache.hadoop.mapreduce.security.SecureShuffleUtils;
import org.apache.hadoop.mapreduce.task.reduce.ShuffleHeader;
-import org.eclipse.jetty.http.HttpHeader;
import static io.netty.buffer.Unpooled.wrappedBuffer;
import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE;
@@ -128,6 +127,15 @@
*
*/
public class ShuffleChannelHandler extends SimpleChannelInboundHandler
+ * Some servlets have to be set up while their context is still stopped -
+ * a Jetty WebSocket servlet needs the WebSocket components installed on
+ * the context, and installing them once the context has started throws.
+ * Builder gives no other window between build and start, and the work is
+ * specific enough to a single webapp that pulling its dependencies into
+ * this module would be the wrong trade.
+ *
+ * @param configurer work to run on the server before it starts
+ * @return this builder
+ */
+ public Builder
+ * What is asserted is the handshake, not a working shell: there is no
+ * container "abc" here, so ContainerShellWebSocket closes the session
+ * shortly after it opens. Getting a Session at all is the interesting part,
+ * because it is the whole Jetty upgrade path - and it is what breaks, with a
+ * 500, when the WebSocket components are missing from the context.
+ */
@Test
- public void testWebServerWithServlet() {
+ public void testWebServerWithServlet() throws Exception {
int port = startNMWebAppServer("0.0.0.0");
LOG.info("bind to port: " + port);
- StringBuilder sb = new StringBuilder();
- sb.append("ws://localhost:").append(port).append("/container/abc/");
- String dest = sb.toString();
+ String dest = "ws://localhost:" + port + "/container/abc/";
WebSocketClient client = new WebSocketClient();
try {
- ContainerShellClientSocketTest socket = new ContainerShellClientSocketTest();
+ ContainerShellClientSocketTest socket =
+ new ContainerShellClientSocketTest();
client.start();
- URI echoUri = new URI(dest);
- Future