From 59a66158ed0c12dd10cec842b0ccf595cbe2e4eb Mon Sep 17 00:00:00 2001
From: Greg Wilkins <gregw@webtide.com>
Date: Mon, 11 May 2020 22:09:19 +0200
Subject: [PATCH] Issue #4860 NPE HttpFields

Fixes from review.
Fixed iterator overflow bug
clearer updates of size
better nonNull messages

Signed-off-by: Greg Wilkins <gregw@webtide.com>
---
 .../org/eclipse/jetty/http/HttpFields.java     | 18 +++++++++++-------
 .../org/eclipse/jetty/http/HttpFieldsTest.java |  2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java
index bd3171a7591..019b8601fc8 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java
@@ -636,7 +636,8 @@ public class HttpFields implements Iterable<HttpField>
             {
                 if (put)
                 {
-                    System.arraycopy(_fields, i + 1, _fields, i, --_size - i);
+                    _size--;
+                    System.arraycopy(_fields, i + 1, _fields, i, _size - i);
                 }
                 else
                 {
@@ -676,7 +677,7 @@ public class HttpFields implements Iterable<HttpField>
      */
     public void put(HttpHeader header, String value)
     {
-        Objects.requireNonNull(header, "header");
+        Objects.requireNonNull(header, "header must not be null");
 
         if (value == null)
             remove(header);
@@ -692,7 +693,7 @@ public class HttpFields implements Iterable<HttpField>
      */
     public void put(String name, List<String> list)
     {
-        Objects.requireNonNull(name, "name");
+        Objects.requireNonNull(name, "name must not be null");
 
         remove(name);
         if (list == null)
@@ -736,7 +737,7 @@ public class HttpFields implements Iterable<HttpField>
      */
     public void add(HttpHeader header, String value)
     {
-        Objects.requireNonNull(header, "header");
+        Objects.requireNonNull(header, "header must not be null");
 
         if (value == null)
             throw new IllegalArgumentException("null value");
@@ -760,7 +761,8 @@ public class HttpFields implements Iterable<HttpField>
             if (f.getHeader() == name)
             {
                 removed = f;
-                System.arraycopy(_fields, i + 1, _fields, i, --_size - i);
+                _size--;
+                System.arraycopy(_fields, i + 1, _fields, i, _size - i);
             }
         }
         return removed;
@@ -781,7 +783,8 @@ public class HttpFields implements Iterable<HttpField>
             if (f.getName().equalsIgnoreCase(name))
             {
                 removed = f;
-                System.arraycopy(_fields, i + 1, _fields, i, --_size - i);
+                _size--;
+                System.arraycopy(_fields, i + 1, _fields, i, _size - i);
             }
         }
         return removed;
@@ -1229,8 +1232,9 @@ public class HttpFields implements Iterable<HttpField>
             if (field != null)
             {
                 _fields = Arrays.copyOf(_fields, _fields.length + 1);
-                System.arraycopy(_fields, _cursor, _fields, _cursor + 1, _size++);
+                System.arraycopy(_fields, _cursor, _fields, _cursor + 1, _size - _cursor);
                 _fields[_cursor++] = field;
+                _size++;
                 _current = -1;
             }
         }
diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java
index 8d10cbd6e67..b24b623a3ba 100644
--- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java
+++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java
@@ -767,7 +767,7 @@ public class HttpFieldsTest
     @Test
     public void testIteration()
     {
-        HttpFields header = new HttpFields();
+        HttpFields header = new HttpFields(5);
         Iterator<HttpField> i = header.iterator();
         assertThat(i.hasNext(), is(false));