8333590: UnmodifiableHeaders.toString() returns a value that represents empty headers

Reviewed-by: dfuchs, michaelm

Author: jaikiran

src/jdk.httpserver/share/classes/sun/net/httpserver/UnmodifiableHeaders.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -32,73 +32,96 @@
 public class UnmodifiableHeaders extends Headers {
 
     private final Headers headers;  // modifiable, but no reference to it escapes
-    private final Map<String, List<String>> map;  // unmodifiable
+    private final Map<String, List<String>> unmodifiableView;  // unmodifiable
 
     public UnmodifiableHeaders(Headers headers) {
         var h = headers;
         var unmodHeaders = new Headers();
         h.forEach((k, v) -> unmodHeaders.put(k, Collections.unmodifiableList(v)));
-        this.map = Collections.unmodifiableMap(unmodHeaders);
+        this.unmodifiableView = Collections.unmodifiableMap(unmodHeaders);
         this.headers = unmodHeaders;
     }
 
+    @Override
     public int size() {return headers.size();}
 
+    @Override
     public boolean isEmpty() {return headers.isEmpty();}
 
+    @Override
     public boolean containsKey(Object key) { return headers.containsKey(key); }
 
+    @Override
     public boolean containsValue(Object value) { return headers.containsValue(value); }
 
+    @Override
     public List<String> get(Object key) { return headers.get(key); }
 
+    @Override
     public String getFirst(String key) { return headers.getFirst(key); }
 
+    @Override
     public List<String> put(String key, List<String> value) {
         throw new UnsupportedOperationException ("unsupported operation");
     }
 
+    @Override
     public void add(String key, String value) {
         throw new UnsupportedOperationException ("unsupported operation");
     }
 
+    @Override
     public void set(String key, String value) {
         throw new UnsupportedOperationException ("unsupported operation");
     }
 
+    @Override
     public List<String> remove(Object key) {
         throw new UnsupportedOperationException ("unsupported operation");
     }
 
+    @Override
     public void putAll(Map<? extends String,? extends List<String>> t)  {
         throw new UnsupportedOperationException ("unsupported operation");
     }
 
+    @Override
     public void clear() {
         throw new UnsupportedOperationException ("unsupported operation");
     }
 
-    public Set<String> keySet() { return map.keySet(); }
+    @Override
+    public Set<String> keySet() { return unmodifiableView.keySet(); }
 
-    public Collection<List<String>> values() { return map.values(); }
+    @Override
+    public Collection<List<String>> values() { return unmodifiableView.values(); }
 
-    /* TODO check that contents of set are not modifable : security */
-
-    public Set<Map.Entry<String, List<String>>> entrySet() { return map.entrySet(); }
+    @Override
+    public Set<Map.Entry<String, List<String>>> entrySet() { return unmodifiableView.entrySet(); }
 
+    @Override
     public List<String> replace(String key, List<String> value) {
         throw new UnsupportedOperationException("unsupported operation");
     }
 
+    @Override
     public boolean replace(String key, List<String> oldValue, List<String> newValue) {
         throw new UnsupportedOperationException ("unsupported operation");
     }
 
+    @Override
     public void replaceAll(BiFunction<? super String, ? super List<String>, ? extends List<String>> function) {
         throw new UnsupportedOperationException ("unsupported operation");
     }
 
+    @Override
     public boolean equals(Object o) {return headers.equals(o);}
 
+    @Override
     public int hashCode() {return headers.hashCode();}
+
+    @Override
+    public String toString() {
+        return headers.toString();
+    }
 }

test/jdk/com/sun/net/httpserver/UnmodifiableHeadersTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8251496
+ * @bug 8251496 8333590
  * @summary Test that UnmodifiableHeaders is in fact immutable
  * @modules jdk.httpserver/sun.net.httpserver:+open
  * @run testng/othervm UnmodifiableHeadersTest
@@ -44,7 +44,9 @@
 import org.testng.annotations.Test;
 import sun.net.httpserver.UnmodifiableHeaders;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertThrows;
+import static org.testng.Assert.assertTrue;
 
 public class UnmodifiableHeadersTest {
 
@@ -70,9 +72,9 @@ public Object[][] headers() {
         var exchange = new TestHttpExchange(headers);
 
         return new Object[][] {
-            { exchange.getRequestHeaders() },
-            { Headers.of("Foo", "Bar") },
-            { Headers.of(Map.of("Foo", List.of("Bar"))) },
+                { exchange.getRequestHeaders() },
+                { Headers.of("Foo", "Bar") },
+                { Headers.of(Map.of("Foo", List.of("Bar"))) },
         };
     }
 
@@ -83,6 +85,44 @@ public static void testUnmodifiableHeaders(Headers headers) {
         assertUnmodifiableList(headers);
     }
 
+    @DataProvider
+    public Object[][] toStringHeaders() {
+        final Headers headers = new Headers();
+        headers.add("hello", "World");
+        return new Object[][] {
+                { headers },
+                { Headers.of("abc", "XYZ") },
+                { Headers.of(Map.of("foo", List.of("Bar"))) },
+                { Headers.of(Map.of("Hello", List.of())) },
+                { Headers.of(Map.of("one", List.of("two", "THREE"))) },
+        };
+    }
+
+    /*
+     * Verify that the String returned by Headers.toString() contains the expected
+     * key/value(s)
+     */
+    @Test(dataProvider = "toStringHeaders")
+    public void testToString(final Headers headers) {
+        final Headers copy = Headers.of(headers);
+        assertNotNull(copy, "Headers.of() returned null");
+        final String actualToString = copy.toString();
+        assertNotNull(actualToString, "toString() returned null");
+        for (final Map.Entry<String, List<String>> originalHeadersEntry : headers.entrySet()) {
+            final String expectedKey = originalHeadersEntry.getKey();
+            // We just verify the presence of key and value in the toString()
+            // return value. We intentionally don't expect or verify that the
+            // toString() content is in some specific form.
+            assertTrue(actualToString.contains(expectedKey),
+                    expectedKey + " missing in output of Headers.of().toString()");
+            final List<String> expectedVals = originalHeadersEntry.getValue();
+            for (final String val : expectedVals) {
+                assertTrue(actualToString.contains(val), val + " for header key "
+                        + expectedKey + " missing in output of Headers.of().toString()");
+            }
+        }
+    }
+
     static final Class<UnsupportedOperationException> UOP = UnsupportedOperationException.class;
 
     static void assertUnsupportedOperation(Headers headers) {