brumi1024 commented on code in PR #8665: URL: https://github.com/apache/hadoop/pull/8665#discussion_r3958732611
########## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/mcp/McpHttpServlet.java: ########## @@ -0,0 +1,111 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.mcp; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.Map; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.commons.io.IOUtils; +import org.apache.commons.io.input.BoundedInputStream; +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +/** + * javax.servlet endpoint implementing MCP streamable HTTP transport. + */ [email protected] [email protected] +public final class McpHttpServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + private final ObjectMapper objectMapper; + private final McpRequestHandler requestHandler; + + McpHttpServlet(ObjectMapper objectMapper, McpRequestHandler requestHandler) { + this.objectMapper = objectMapper; + this.requestHandler = requestHandler; + } + + void close() { + // Stateless server; nothing to release. + } + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + resp.setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + req.setCharacterEncoding(StandardCharsets.UTF_8.name()); + resp.setCharacterEncoding(StandardCharsets.UTF_8.name()); + + byte[] body = readRequestBody(req); + if (body.length == 0 || body.length > McpJsonRpc.MAX_REQUEST_BODY_BYTES) { + writeResponse(objectMapper, resp, body.length == 0 + ? requestHandler.parseErrorResponse() + : requestHandler.requestBodyTooLargeResponse()); + return; + } + + JsonNode requestNode; + try { + requestNode = objectMapper.readTree(body); Review Comment: Default Jackson readTree() consumes the first JSON value without rejecting trailing content. This means concatenating multiple request objects will silently execute the first request and ignore the rest. Requiring complete-body parsing, for example with a reader configured with FAIL_ON_TRAILING_TOKENS would disallow this. ########## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/mcp/McpHttpServer.java: ########## @@ -0,0 +1,272 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.mcp; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.security.GeneralSecurityException; + +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.http.HttpServer2; +import org.apache.hadoop.security.ssl.SSLFactory; +import org.apache.hadoop.util.StringUtils; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.SecureRequestCustomizer; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.SslConnectionFactory; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Minimal Jetty HTTP/HTTPS server that hosts an {@link McpServer} servlet on a dedicated port. + * + * <p>No Kerberos/SPNEGO filters are installed. Callers authenticate MCP tool calls + * at the application layer (for example API keys). + */ [email protected] [email protected] +public final class McpHttpServer implements AutoCloseable { + + private static final Logger LOG = LoggerFactory.getLogger(McpHttpServer.class); + + private final McpServer mcpServer; + private final Server server; + private final ServerConnector connector; + private final SSLFactory sslFactory; + + private McpHttpServer(McpServer mcpServer, Server server, ServerConnector connector, + SSLFactory sslFactory) { + this.mcpServer = mcpServer; + this.server = server; + this.connector = connector; + this.sslFactory = sslFactory; + } + + /** + * Starts a dedicated MCP HTTPS server. + * + * @param mcpServer MCP server whose servlet handles JSON-RPC requests + * @param conf daemon configuration containing SSL keystore settings + * @param bindAddress address to bind (port {@code 0} picks an ephemeral port) + * @param pathSpec servlet path, for example {@code /ws/v1/mcp} + */ + public static McpHttpServer start(McpServer mcpServer, Configuration conf, + InetSocketAddress bindAddress, String pathSpec) throws IOException { + return start(mcpServer, conf, bindAddress, pathSpec, true); + } + + /** + * Starts a dedicated MCP HTTP or HTTPS server. + * + * @param mcpServer MCP server whose servlet handles JSON-RPC requests + * @param conf daemon configuration; SSL keystore settings are required when {@code useHttps} + * is {@code true} + * @param bindAddress address to bind (port {@code 0} picks an ephemeral port) + * @param pathSpec servlet path, for example {@code /ws/v1/mcp} + * @param useHttps when {@code true}, serve HTTPS using the daemon SSL keystore; otherwise + * serve plain HTTP + */ + public static McpHttpServer start(McpServer mcpServer, Configuration conf, + InetSocketAddress bindAddress, String pathSpec, boolean useHttps) throws IOException { + Configuration serverConf = conf == null ? new Configuration(false) : conf; + SSLFactory sslFactory = null; + Server server = new Server(); + ServerConnector connector; + if (useHttps) { + sslFactory = new SSLFactory(SSLFactory.Mode.SERVER, serverConf); + try { + sslFactory.init(); + } catch (GeneralSecurityException e) { + throw new IOException("Failed to initialize SSLFactory", e); + } + connector = createHttpsConnector(server, serverConf, bindAddress, sslFactory); + } else { + connector = createHttpConnector(server, bindAddress); + } + server.addConnector(connector); + + ServletContextHandler context = + new ServletContextHandler(ServletContextHandler.NO_SESSIONS); + context.setContextPath("/"); + context.addServlet(new ServletHolder(mcpServer.getServlet()), pathSpec); + server.setHandler(context); + + try { + server.start(); + } catch (Exception e) { + if (sslFactory != null) { + sslFactory.destroy(); + } Review Comment: Nit: if for example the port is occupied Jetty can throw an Exception after starting, and then nothing will stop the server. It might be a good idea to stop it in the catch phase. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
