Fixes HttpFieldsMap so that now HttpFieldsMap.Mutable.get(name).add(v…

…alue) is supported and correctly backed by the underlying HttpFields instance.

Also made List returned by HttpFieldsMap.Mutable.remove() and HttpFieldsMap.Immutable.get() unmodifiable.

Added test cases.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFieldsMap.java

@@ -13,8 +13,10 @@
 
 package org.eclipse.jetty.http;
 
+import java.util.AbstractList;
 import java.util.AbstractMap;
 import java.util.AbstractSet;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
@@ -24,6 +26,7 @@
 
 /**
  * <p>A {@link java.util.Map} which is backed by an instance of {@link HttpFields.Mutable}.</p>
+ *
  * @see HttpFieldsMap.Mutable
  * @see HttpFieldsMap.Immutable
  */
@@ -46,7 +49,32 @@ public Mutable(HttpFields.Mutable httpFields)
         public List<String> get(Object key)
         {
             if (key instanceof String s)
-                return httpFields.getValuesList(s);
+            {
+                return new AbstractList<>()
+                {
+                    private final List<String> values = httpFields.getValuesList(s);
+
+                    @Override
+                    public String get(int index)
+                    {
+                        return values.get(index);
+                    }
+
+                    @Override
+                    public int size()
+                    {
+                        return values.size();
+                    }
+
+                    @Override
+                    public boolean add(String value)
+                    {
+                        values.add(value);
+                        httpFields.add(s, value);
+                        return true;
+                    }
+                };
+            }
             return null;
         }
 
@@ -65,7 +93,7 @@ public List<String> remove(Object key)
             {
                 List<String> oldValue = get(s);
                 httpFields.remove(s);
-                return oldValue;
+                return Collections.unmodifiableList(oldValue);
             }
             return null;
         }
@@ -118,7 +146,7 @@ public int size()
     }
 
     /**
-     * <p>A {@link java.util.Map} which is backed by an instance of {@link HttpFields.Mutable}.</p>
+     * <p>A {@link java.util.Map} which is backed by an instance of {@link HttpFields}.</p>
      * <p>Any attempt to modify the map will throw {@link UnsupportedOperationException}.</p>
      */
     public static class Immutable extends HttpFieldsMap
@@ -134,7 +162,7 @@ public Immutable(HttpFields httpFields)
         public List<String> get(Object key)
         {
             if (key instanceof String s)
-                return httpFields.getValuesList(s);
+                return Collections.unmodifiableList(httpFields.getValuesList(s));
             return null;
         }
 

jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsMapTest.java

@@ -0,0 +1,113 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.http;
+
+import java.util.List;
+import java.util.Map;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasEntry;
+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.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class HttpFieldsMapTest
+{
+    @Test
+    public void testPutField()
+    {
+        HttpFields.Mutable fields = HttpFields.build().put("foo", "bar");
+        Map<String, List<String>> map = HttpFields.asMap(fields);
+
+        map.put("baz", List.of("qux"));
+
+        assertEquals(2, map.size());
+        assertEquals(2, fields.size());
+        assertThat(map.get("foo"), equalTo(List.of("bar")));
+        assertThat(map.get("baz"), equalTo(List.of("qux")));
+        assertThat(map, hasEntry("baz", List.of("qux")));
+        assertTrue(fields.contains("foo", "bar"));
+        assertTrue(fields.contains("baz", "qux"));
+    }
+
+    @Test
+    public void testPutReplaceField()
+    {
+        HttpFields.Mutable fields = HttpFields.build().put("foo", "bar");
+        Map<String, List<String>> map = HttpFields.asMap(fields);
+
+        List<String> put = map.put("foo", List.of("baz"));
+
+        assertNotNull(put);
+        assertEquals(1, map.size());
+        assertEquals(1, fields.size());
+        assertThat(put, equalTo(List.of("bar")));
+        assertThat(map.get("foo"), equalTo(List.of("baz")));
+        assertFalse(fields.contains("foo", "bar"));
+        assertTrue(fields.contains("foo", "baz"));
+    }
+
+    @Test
+    public void testRemoveField()
+    {
+        HttpFields.Mutable fields = HttpFields.build().put("foo", "bar");
+        Map<String, List<String>> map = HttpFields.asMap(fields);
+
+        List<String> values = map.remove("foo");
+
+        assertThat(values, equalTo(List.of("bar")));
+        assertTrue(map.isEmpty());
+        assertEquals(0, fields.size());
+
+        // Adding to the values is unsupported.
+        assertThrows(UnsupportedOperationException.class, () -> values.add("baz"));
+    }
+
+    @Test
+    public void testAddValue()
+    {
+        HttpFields.Mutable fields = HttpFields.build().put("foo", List.of("bar"));
+        Map<String, List<String>> map = HttpFields.asMap(fields);
+
+        List<String> values = map.get("foo");
+        boolean added = values.add("baz");
+
+        assertTrue(added);
+        assertThat(values, equalTo(List.of("bar", "baz")));
+        assertThat(map.get("foo"), equalTo(values));
+        assertTrue(fields.contains("foo", "bar"));
+        assertTrue(fields.contains("foo", "baz"));
+        assertThat(fields.getValuesList("foo"), equalTo(values));
+    }
+
+    @Test
+    public void testRemoveValue()
+    {
+        HttpFields.Mutable fields = HttpFields.build()
+            .add("foo", "bar")
+            .add("foo", "baz");
+        Map<String, List<String>> map = HttpFields.asMap(fields);
+
+        List<String> values = map.get("foo");
+        assertEquals(2, values.size());
+
+        // Removing from the values is unsupported.
+        assertThrows(UnsupportedOperationException.class, () -> values.remove("bar"));
+    }
+}