Address additional code review feedback

- Extracted query parsing logic to parseQueryParams method in URITemplateProxyServlet
- Changed extractHost from private to protected in ProxyServlet to expose for subclasses
- Inlined extractHost call in URITemplateProxyServlet (removed intermediate variable)
- Simplified RequestInfoServlet to use transferTo() instead of manual buffer copying
- Simplified testRedirect to use replaceTargetServlet helper method
- Improved replaceTargetServlet to use delegating servlet pattern (no server restart needed)

Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com>

Author: Copilot

src/main/java/org/mitre/dsmiley/httpproxy/ProxyServlet.java

@@ -223,7 +223,7 @@ protected void initTarget() throws ServletException {
   /**
    * Extract host:port from URI
    */
-  private String extractHost(URI uri) {
+  protected String extractHost(URI uri) {
     String host = uri.getHost();
     int port = uri.getPort();
     if (port == -1) {

src/main/java/org/mitre/dsmiley/httpproxy/URITemplateProxyServlet.java

@@ -91,25 +91,7 @@ protected void service(HttpServletRequest servletRequest, HttpServletResponse se
       queryString = queryString.substring(0, hash);
     }
 
-    LinkedHashMap<String, String> params = new LinkedHashMap<String, String>();
-    if (requestQueryString != null && !requestQueryString.isEmpty()) {
-      // Parse query string manually
-      String[] pairs = requestQueryString.split("&");
-      for (String pair : pairs) {
-        int idx = pair.indexOf("=");
-        if (idx > 0) {
-          String key = URLDecoder.decode(pair.substring(0, idx), StandardCharsets.UTF_8);
-          String value = URLDecoder.decode(pair.substring(idx + 1), StandardCharsets.UTF_8);
-          params.put(key, value);
-        } else if (idx == 0) {
-          // Ignore malformed parameter
-        } else {
-          // Parameter without value
-          String key = URLDecoder.decode(pair, StandardCharsets.UTF_8);
-          params.put(key, "");
-        }
-      }
-    }
+    LinkedHashMap<String, String> params = parseQueryParams(requestQueryString);
 
     //Now rewrite the URL
     StringBuffer urlBuf = new StringBuffer();//note: StringBuilder isn't supported by Matcher
@@ -131,8 +113,7 @@ protected void service(HttpServletRequest servletRequest, HttpServletResponse se
     } catch (Exception e) {
       throw new ServletException("Rewritten targetUri is invalid: " + newTargetUri,e);
     }
-    String targetHost = extractHost(targetUriObj);
-    servletRequest.setAttribute(ATTR_TARGET_HOST, targetHost);
+    servletRequest.setAttribute(ATTR_TARGET_HOST, extractHost(targetUriObj));
 
     //Determine the new query string based on removing the used names
     StringBuilder newQueryBuf = new StringBuilder(queryString.length());
@@ -154,14 +135,28 @@ protected String rewriteQueryStringFromRequest(HttpServletRequest servletRequest
   }
 
   /**
-   * Extract host:port from URI (helper method)
+   * Parse query string into a map of key-value pairs
    */
-  private String extractHost(URI uri) {
-    String host = uri.getHost();
-    int port = uri.getPort();
-    if (port == -1) {
-      return host;
+  private LinkedHashMap<String, String> parseQueryParams(String requestQueryString) {
+    LinkedHashMap<String, String> params = new LinkedHashMap<String, String>();
+    if (requestQueryString != null && !requestQueryString.isEmpty()) {
+      // Parse query string manually
+      String[] pairs = requestQueryString.split("&");
+      for (String pair : pairs) {
+        int idx = pair.indexOf("=");
+        if (idx > 0) {
+          String key = URLDecoder.decode(pair.substring(0, idx), StandardCharsets.UTF_8);
+          String value = URLDecoder.decode(pair.substring(idx + 1), StandardCharsets.UTF_8);
+          params.put(key, value);
+        } else if (idx == 0) {
+          // Ignore malformed parameter
+        } else {
+          // Parameter without value
+          String key = URLDecoder.decode(pair, StandardCharsets.UTF_8);
+          params.put(key, "");
+        }
+      }
     }
-    return host + ":" + port;
+    return params;
   }
 }

src/test/java/org/mitre/dsmiley/httpproxy/ProxyServletTest.java

@@ -71,6 +71,7 @@ public class ProxyServletTest
    */
   protected Server targetServer;
   protected int targetServerPort;
+  protected DelegatingServlet delegatingServlet;
 
   /** From Meterware httpunit. */
   protected ServletRunner servletRunner;
@@ -84,11 +85,12 @@ public class ProxyServletTest
 
   @Before
   public void setUp() throws Exception {
-    // Start Jetty server as target
+    // Start Jetty server as target with a delegating servlet that can be updated
     targetServer = new Server(0);
     ServletHandler handler = new ServletHandler();
     targetServer.setHandler(handler);
-    handler.addServletWithMapping(RequestInfoServlet.class, "/targetPath/*");
+    delegatingServlet = new DelegatingServlet(new RequestInfoServlet());
+    handler.addServletWithMapping(new ServletHolder(delegatingServlet), "/targetPath/*");
     targetServer.start();
     targetServerPort = ((ServerConnector) targetServer.getConnectors()[0]).getLocalPort();
 
@@ -106,16 +108,30 @@ public void setUp() throws Exception {
   }
 
   /**
-   * Helper to restart target server with a custom servlet
+   * Helper to replace the target servlet handler without restarting the server
    */
-  protected void replaceTargetServlet(HttpServlet servlet) throws Exception {
-    targetServer.stop();
-    targetServer = new Server(targetServerPort);
-    ServletHandler handler = new ServletHandler();
-    targetServer.setHandler(handler);
-    ServletHolder holder = new ServletHolder(servlet);
-    handler.addServletWithMapping(holder, "/targetPath/*");
-    targetServer.start();
+  protected void replaceTargetServlet(HttpServlet servlet) {
+    delegatingServlet.setDelegate(servlet);
+  }
+  
+  /**
+   * Delegating servlet that can have its delegate updated at runtime
+   */
+  protected static class DelegatingServlet extends HttpServlet {
+    private HttpServlet delegate;
+    
+    public DelegatingServlet(HttpServlet delegate) {
+      this.delegate = delegate;
+    }
+    
+    public void setDelegate(HttpServlet delegate) {
+      this.delegate = delegate;
+    }
+    
+    @Override
+    protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+      delegate.service(req, resp);
+    }
   }
 
   protected void setUpServlet(Properties servletProps) {
@@ -175,14 +191,7 @@ public void testPost() throws Exception {
   public void testRedirect() throws Exception {
     final String COOKIE_SET_HEADER = "Set-Cookie";
 
-    // Stop the target server and restart with redirect servlet
-    targetServer.stop();
-    targetServer = new Server(targetServerPort);
-    ServletHandler handler = new ServletHandler();
-    targetServer.setHandler(handler);
-    
-    // Add a redirect servlet
-    ServletHolder redirectHolder = new ServletHolder(new HttpServlet() {
+    replaceTargetServlet(new HttpServlet() {
       @Override
       protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
         String targetHeader = request.getHeader("xxTarget");
@@ -193,8 +202,6 @@ protected void service(HttpServletRequest request, HttpServletResponse response)
         }
       }
     });
-    handler.addServletWithMapping(redirectHolder, "/targetPath/*");
-    targetServer.start();
 
     GetMethodWebRequest request = makeGetMethodRequest(sourceBaseUri + "/%64%69%72%2F");
     assertRedirect(request, "/dummy", "/dummy");//TODO represents a bug to fix
@@ -733,12 +740,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response)
       pw.flush();//done with pw now
 
       // Copy request body
-      InputStream is = request.getInputStream();
-      byte[] buffer = new byte[8192];
-      int bytesRead;
-      while ((bytesRead = is.read(buffer)) != -1) {
-        baos.write(buffer, 0, bytesRead);
-      }
+      request.getInputStream().transferTo(baos);
 
       response.setStatus(200);
       response.setHeader("X-Reason", "TESTREASON");