From f1a13521df66ce4206b8797707d176c4b3cd972b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 13 Aug 2020 10:06:31 +0200 Subject: [PATCH 1/3] improved implementation Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/http/HttpFields.java | 79 ++++++++++++------- 1 file changed, 51 insertions(+), 28 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 cca38cd2c5c..c0b9aa7d907 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 @@ -166,36 +166,55 @@ public class HttpFields implements Iterable */ public void computeField(String name, BiFunction, HttpField> computeFn) { - boolean found = false; - ListIterator iterator = listIterator(); - while (iterator.hasNext()) + // Look for first occurrence + int i = 0; + int first = -1; + for (; i < _size; i++) { - HttpField field = iterator.next(); - if (field.getName().equalsIgnoreCase(name)) + HttpField f = _fields[i++]; + if (f.getName().equalsIgnoreCase(name)) { - if (found) - { - // Remove other headers with the same name, since - // we have computed one from all of them already. - iterator.remove(); - } - else - { - found = true; - HttpField newField = computeFn.apply(name, Collections.unmodifiableList(getFields(name))); - if (newField == null) - iterator.remove(); - else - iterator.set(newField); - } + first = i++; + break; } } - if (!found) + + // If the header is not found, add a new one; + if (first < 0) { HttpField newField = computeFn.apply(name, null); if (newField != null) - put(newField); + add(newField); + return; } + + // Are there any more occurrences? + List found = null; + for (; i < _size; i++) + { + HttpField f = _fields[i++]; + if (f.getName().equalsIgnoreCase(name)) + { + if (found == null) + { + found = new ArrayList<>(); + found.add(_fields[first]); + } + // Remember and remove additional fields + found.add(f); + remove(i); + } + } + + // If no additional fields were found, handle singleton case + if (found == null) + found = Collections.singletonList(_fields[first]); + + HttpField newField = computeFn.apply(name, found); + if (newField == null) + remove(first); + else + _fields[first] = newField; } public int size() @@ -887,8 +906,7 @@ public class HttpFields implements Iterable if (f.getHeader() == name) { removed = f; - _size--; - System.arraycopy(_fields, i + 1, _fields, i, _size - i); + remove(i); } } return removed; @@ -909,13 +927,19 @@ public class HttpFields implements Iterable if (f.getName().equalsIgnoreCase(name)) { removed = f; - _size--; - System.arraycopy(_fields, i + 1, _fields, i, _size - i); + remove(i); } } return removed; } + private void remove(int i) + { + _size--; + System.arraycopy(_fields, i + 1, _fields, i, _size - i); + _fields[_size] = null; + } + /** * Get a header as an long value. Returns the value of an integer field or -1 if not found. The * case of the field name is ignored. @@ -1307,8 +1331,7 @@ public class HttpFields implements Iterable { if (_current < 0) throw new IllegalStateException(); - _size--; - System.arraycopy(_fields, _current + 1, _fields, _current, _size - _current); + HttpFields.this.remove(_current); _fields[_size] = null; _cursor = _current; _current = -1; From f31604cff2206be10b329adc1561b379fa1fb99e Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 13 Aug 2020 11:17:19 +0200 Subject: [PATCH 2/3] added test. Fixed bugs Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/http/HttpField.java | 5 +++ .../org/eclipse/jetty/http/HttpFields.java | 32 +++++++++---------- .../eclipse/jetty/http/HttpFieldsTest.java | 28 ++++++++++++++++ 3 files changed, 49 insertions(+), 16 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java index e04d7e2345e..f0bf6f0c1c1 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java @@ -292,6 +292,11 @@ public class HttpField return _name.equalsIgnoreCase(field.getName()); } + public boolean is(String name) + { + return _name.equalsIgnoreCase(name); + } + private int nameHashCode() { int h = this.hash; 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 c0b9aa7d907..4837db945cb 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 @@ -171,8 +171,8 @@ public class HttpFields implements Iterable int first = -1; for (; i < _size; i++) { - HttpField f = _fields[i++]; - if (f.getName().equalsIgnoreCase(name)) + HttpField f = _fields[i]; + if (f.is(name)) { first = i++; break; @@ -192,8 +192,8 @@ public class HttpFields implements Iterable List found = null; for (; i < _size; i++) { - HttpField f = _fields[i++]; - if (f.getName().equalsIgnoreCase(name)) + HttpField f = _fields[i]; + if (f.is(name)) { if (found == null) { @@ -296,7 +296,7 @@ public class HttpFields implements Iterable for (int i = 0; i < _size; i++) { HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(name)) + if (f.is(name)) return f; } return null; @@ -324,7 +324,7 @@ public class HttpFields implements Iterable for (int i = 0; i < _size; i++) { HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(name)) + if (f.is(name)) { if (fields == null) fields = new ArrayList<>(); @@ -361,7 +361,7 @@ public class HttpFields implements Iterable for (int i = _size; i-- > 0; ) { HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(name) && f.contains(value)) + if (f.is(name) && f.contains(value)) return true; } return false; @@ -383,7 +383,7 @@ public class HttpFields implements Iterable for (int i = _size; i-- > 0; ) { HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(name)) + if (f.is(name)) return true; } return false; @@ -417,7 +417,7 @@ public class HttpFields implements Iterable for (int i = 0; i < _size; i++) { HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(header)) + if (f.is(header)) return f.getValue(); } return null; @@ -453,7 +453,7 @@ public class HttpFields implements Iterable for (int i = 0; i < _size; i++) { HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(name)) + if (f.is(name)) { if (list == null) list = new ArrayList<>(size() - i); @@ -508,7 +508,7 @@ public class HttpFields implements Iterable for (int i = 0; i < _size; i++) { HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(name)) + if (f.is(name)) { if (existing == null) existing = new QuotedCSV(false); @@ -596,7 +596,7 @@ public class HttpFields implements Iterable QuotedCSV values = null; for (HttpField f : this) { - if (f.getName().equalsIgnoreCase(name)) + if (f.is(name)) { if (values == null) values = new QuotedCSV(keepQuotes); @@ -654,7 +654,7 @@ public class HttpFields implements Iterable QuotedQualityCSV values = null; for (HttpField f : this) { - if (f.getName().equalsIgnoreCase(name)) + if (f.is(name)) { if (values == null) values = new QuotedQualityCSV(); @@ -676,7 +676,7 @@ public class HttpFields implements Iterable { final HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(name) && f.getValue() != null) + if (f.is(name) && f.getValue() != null) { final int first = i; return new Enumeration() @@ -692,7 +692,7 @@ public class HttpFields implements Iterable while (i < _size) { field = _fields[i++]; - if (field.getName().equalsIgnoreCase(name) && field.getValue() != null) + if (field.is(name) && field.getValue() != null) return true; } field = null; @@ -924,7 +924,7 @@ public class HttpFields implements Iterable for (int i = _size; i-- > 0; ) { HttpField f = _fields[i]; - if (f.getName().equalsIgnoreCase(name)) + if (f.is(name)) { removed = f; remove(i); 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 8ad677a9f27..bb9eb90826d 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 @@ -866,4 +866,32 @@ public class HttpFieldsTest assertThat(header.stream().count(), is(3L)); assertThat(header.stream().map(HttpField::getName).filter("name2"::equalsIgnoreCase).count(), is(1L)); } + + @Test + public void testCompute() + { + HttpFields header = new HttpFields(); + assertThat(header.size(), is(0)); + + header.computeField("Test", (n, f) -> null); + assertThat(header.size(), is(0)); + + header.add(new HttpField("Before", "value")); + assertThat(header.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Before: value")); + + header.computeField("Test", (n, f) -> new HttpField(n, "one")); + assertThat(header.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Before: value", "Test: one")); + + header.add(new HttpField("After", "value")); + assertThat(header.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Before: value", "Test: one", "After: value")); + + header.add(new HttpField("Test", "extra")); + assertThat(header.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Before: value", "Test: one", "After: value", "Test: extra")); + + header.computeField("Test", (n, f) -> new HttpField("TEST", "count=" + f.size())); + assertThat(header.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Before: value", "TEST: count=2", "After: value")); + + header.computeField("TEST", (n, f) -> null); + assertThat(header.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Before: value", "After: value")); + } } From a933b1645cce93728bf195462930edbd0b5a91a8 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 13 Aug 2020 11:27:37 +0200 Subject: [PATCH 3/3] simplified loops unmodifiable found list Signed-off-by: Greg Wilkins --- .../src/main/java/org/eclipse/jetty/http/HttpFields.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 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 4837db945cb..21882181bd0 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 @@ -167,14 +167,13 @@ public class HttpFields implements Iterable public void computeField(String name, BiFunction, HttpField> computeFn) { // Look for first occurrence - int i = 0; int first = -1; - for (; i < _size; i++) + for (int i = 0; i < _size; i++) { HttpField f = _fields[i]; if (f.is(name)) { - first = i++; + first = i; break; } } @@ -190,7 +189,7 @@ public class HttpFields implements Iterable // Are there any more occurrences? List found = null; - for (; i < _size; i++) + for (int i = first + 1; i < _size; i++) { HttpField f = _fields[i]; if (f.is(name)) @@ -209,6 +208,8 @@ public class HttpFields implements Iterable // If no additional fields were found, handle singleton case if (found == null) found = Collections.singletonList(_fields[first]); + else + found = Collections.unmodifiableList(found); HttpField newField = computeFn.apply(name, found); if (newField == null)