diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java index eaf96ccb5b87..07251165e885 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java @@ -32,8 +32,9 @@ import static org.eclipse.jetty.util.UrlEncoded.decodeHexByte; /** - * A {@link CompletableFuture} that is completed once a {@code application/x-www-form-urlencoded} - * content has been parsed asynchronously from the {@link Content.Source}. + *

A {@link CompletableFuture} that is completed once a {@code application/x-www-form-urlencoded} + * content has been parsed asynchronously from the {@link Content.Source}.

+ *

Specification.

*/ public class FormFields extends ContentSourceCompletableFuture { @@ -209,98 +210,103 @@ private FormFields(Content.Source source, Charset charset, int maxFields, int ma @Override protected Fields parse(Content.Chunk chunk) throws CharacterCodingException { - String value = null; ByteBuffer buffer = chunk.getByteBuffer(); - do + while (BufferUtil.hasContent(buffer)) { - loop: - while (BufferUtil.hasContent(buffer)) + byte b = buffer.get(); + switch (_percent) { - byte b = buffer.get(); - switch (_percent) + case 1 -> { - case 1 -> - { - _percentCode = b; - _percent++; - continue; - } - case 2 -> - { - _builder.append(decodeHexByte((char)_percentCode, (char)b)); - _percent = 0; - continue; - } + _percentCode = b; + _percent++; + continue; + } + case 2 -> + { + _percent = 0; + _builder.append(decodeHexByte((char)_percentCode, (char)b)); + continue; } + } - if (_name == null) + if (_name == null) + { + switch (b) { - switch (b) + case '&' -> { - case '=' -> - { - _name = _builder.build(); - checkLength(_name); - } - case '+' -> _builder.append((byte)' '); - case '%' -> _percent++; - default -> _builder.append(b); + String name = _builder.build(); + checkMaxLength(name); + onNewField(name, ""); } - } - else - { - switch (b) + case '=' -> { - case '&' -> - { - value = _builder.build(); - checkLength(value); - break loop; - } - case '+' -> _builder.append((byte)' '); - case '%' -> _percent++; - default -> _builder.append(b); + _name = _builder.build(); + checkMaxLength(_name); } + case '+' -> _builder.append(' '); + case '%' -> _percent++; + default -> _builder.append(b); } } - - if (_name != null) + else { - if (value == null && chunk.isLast()) + switch (b) { - if (_percent > 0) + case '&' -> { - _builder.append((byte)'%'); - _builder.append(_percentCode); + String value = _builder.build(); + checkMaxLength(value); + onNewField(_name, value); + _name = null; } - value = _builder.build(); - checkLength(value); + case '+' -> _builder.append(' '); + case '%' -> _percent++; + default -> _builder.append(b); } + } + } - if (value != null) - { - Fields.Field field = new Fields.Field(_name, value); - _name = null; - value = null; - if (_maxFields >= 0 && _fields.getSize() >= _maxFields) - throw new IllegalStateException("form with too many fields > " + _maxFields); - _fields.add(field); - } + if (!chunk.isLast()) + return null; + + // Append any remaining %x. + if (_percent > 0) + throw new IllegalStateException("invalid percent encoding"); + String value = _builder.build(); + + if (_name == null) + { + if (!value.isEmpty()) + { + checkMaxLength(value); + onNewField(value, ""); } + return _fields; } - while (BufferUtil.hasContent(buffer)); - return chunk.isLast() ? _fields : null; + checkMaxLength(value); + onNewField(_name, value); + return _fields; } - private void checkLength(String nameOrValue) + private void checkMaxLength(String nameOrValue) { if (_maxLength >= 0) { _length += nameOrValue.length(); if (_length > _maxLength) - throw new IllegalStateException("form too large"); + throw new IllegalStateException("form too large > " + _maxLength); } } + + private void onNewField(String name, String value) + { + Fields.Field field = new Fields.Field(name, value); + _fields.add(field); + if (_maxFields >= 0 && _fields.getSize() > _maxFields) + throw new IllegalStateException("form with too many fields > " + _maxFields); + } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java index cfd0b1ec650c..40b6ff53217b 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java @@ -13,11 +13,13 @@ package org.eclipse.jetty.server; +import java.nio.charset.CharacterCodingException; import java.nio.charset.Charset; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; @@ -32,31 +34,41 @@ import org.junit.jupiter.params.provider.MethodSource; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class FormFieldsTest { - public static Stream tests() + public static Stream validData() { return Stream.of( + Arguments.of(List.of(""), UTF_8, -1, -1, Map.of()), + Arguments.of(List.of("name"), UTF_8, -1, -1, Map.of("name", "")), + Arguments.of(List.of("name&"), UTF_8, -1, -1, Map.of("name", "")), + Arguments.of(List.of("name="), UTF_8, -1, -1, Map.of("name", "")), + Arguments.of(List.of("name%00="), UTF_8, -1, -1, Map.of("name\u0000", "")), + Arguments.of(List.of("n1=v1&n2"), UTF_8, -1, -1, Map.of("n1", "v1", "n2", "")), + Arguments.of(List.of("n1=v1&n2&n3=v3&n4"), UTF_8, -1, -1, Map.of("n1", "v1", "n2", "", "n3", "v3", "n4", "")), Arguments.of(List.of("name=value"), UTF_8, -1, -1, Map.of("name", "value")), + Arguments.of(List.of("name=%0A"), UTF_8, -1, -1, Map.of("name", "\n")), Arguments.of(List.of("name=value", ""), UTF_8, -1, -1, Map.of("name", "value")), Arguments.of(List.of("name", "=value", ""), UTF_8, -1, -1, Map.of("name", "value")), Arguments.of(List.of("n", "ame", "=", "value"), UTF_8, -1, -1, Map.of("name", "value")), Arguments.of(List.of("n=v&X=Y"), UTF_8, 2, 4, Map.of("n", "v", "X", "Y")), Arguments.of(List.of("name=f¤¤&X=Y"), UTF_8, -1, -1, Map.of("name", "f¤¤", "X", "Y")), - Arguments.of(List.of("n=v&X=Y"), UTF_8, 1, -1, null), - Arguments.of(List.of("n=v&X=Y"), UTF_8, -1, 3, null) + Arguments.of(List.of("na+me=", "va", "+", "lue"), UTF_8, -1, -1, Map.of("na me", "va lue")), + Arguments.of(List.of("=v"), UTF_8, -1, -1, Map.of("", "v")) ); } @ParameterizedTest - @MethodSource("tests") - public void testFormFields(List chunks, Charset charset, int maxFields, int maxLength, Map expected) - throws Exception + @MethodSource("validData") + public void testValidFormFields(List chunks, Charset charset, int maxFields, int maxLength, Map expected) { AsyncContent source = new AsyncContent(); Attributes attributes = new Attributes.Mapped(); @@ -66,8 +78,9 @@ public void testFormFields(List chunks, Charset charset, int maxFields, int last = chunks.size() - 1; FutureCallback eof = new FutureCallback(); for (int i = 0; i <= last; i++) + { source.write(i == last, BufferUtil.toBuffer(chunks.get(i), charset), i == last ? eof : Callback.NOOP); - + } try { @@ -89,4 +102,45 @@ public void testFormFields(List chunks, Charset charset, int maxFields, assertNull(expected); } } + + public static Stream invalidData() + { + return Stream.of( + Arguments.of(List.of("%A"), UTF_8, -1, -1, IllegalStateException.class), + Arguments.of(List.of("name%"), UTF_8, -1, -1, IllegalStateException.class), + Arguments.of(List.of("name%A"), UTF_8, -1, -1, IllegalStateException.class), + + // TODO: these 2 should throw the same exception. + Arguments.of(List.of("name%A="), UTF_8, -1, -1, CharacterCodingException.class), + Arguments.of(List.of("name%A&"), UTF_8, -1, -1, IllegalArgumentException.class), + + Arguments.of(List.of("name=%"), UTF_8, -1, -1, IllegalStateException.class), + Arguments.of(List.of("name=A%%A"), UTF_8, -1, -1, IllegalArgumentException.class), + Arguments.of(List.of("name=A%%3D"), UTF_8, -1, -1, IllegalArgumentException.class), + Arguments.of(List.of("%="), UTF_8, -1, -1, IllegalStateException.class), + Arguments.of(List.of("name=%A"), UTF_8, -1, -1, IllegalStateException.class), + Arguments.of(List.of("name=value%A"), UTF_8, -1, -1, IllegalStateException.class), + Arguments.of(List.of("n=v&X=Y"), UTF_8, 1, -1, IllegalStateException.class), + Arguments.of(List.of("n=v&X=Y"), UTF_8, -1, 3, IllegalStateException.class), + Arguments.of(List.of("n%AH=v"), UTF_8, -1, -1, IllegalArgumentException.class), + Arguments.of(List.of("n=v%AH"), UTF_8, -1, -1, IllegalArgumentException.class), + Arguments.of(List.of("n=v%FF"), UTF_8, -1, -1, CharacterCodingException.class) + ); + } + + @ParameterizedTest + @MethodSource("invalidData") + public void testInvalidFormFields(List chunks, Charset charset, int maxFields, int maxLength, Class expectedException) + { + AsyncContent source = new AsyncContent(); + CompletableFuture futureFields = FormFields.from(source, new Attributes.Mapped(), charset, maxFields, maxLength); + assertFalse(futureFields.isDone()); + int last = chunks.size() - 1; + for (int i = 0; i <= last; i++) + { + source.write(i == last, BufferUtil.toBuffer(chunks.get(i)), Callback.NOOP); + } + Throwable cause = assertThrows(ExecutionException.class, futureFields::get).getCause(); + assertThat(cause, instanceOf(expectedException)); + } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CharsetStringBuilder.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CharsetStringBuilder.java index 8ac5b800caf8..0959e7406206 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CharsetStringBuilder.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CharsetStringBuilder.java @@ -95,6 +95,14 @@ default void append(ByteBuffer buf) */ String build() throws CharacterCodingException; + /** + * @return the length in characters + */ + int length(); + + /** + *

Resets this sequence to be empty.

+ */ void reset(); /** @@ -168,6 +176,12 @@ public String build() return s; } + @Override + public int length() + { + return _builder.length(); + } + @Override public void reset() { @@ -207,6 +221,12 @@ public String build() return s; } + @Override + public int length() + { + return _builder.length(); + } + @Override public void reset() { @@ -319,6 +339,12 @@ public String build() throws CharacterCodingException } } + @Override + public int length() + { + return _stringBuilder.length(); + } + @Override public void reset() { diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java index 3da2f322c1c7..77c17b673ffe 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java @@ -92,6 +92,7 @@ protected Utf8StringBuilder(StringBuilder buffer) _buffer = buffer; } + @Override public int length() { return _buffer.length();