diff --git a/media/multipart/src/main/java/io/helidon/media/multipart/MimeParser.java b/media/multipart/src/main/java/io/helidon/media/multipart/MimeParser.java index 250d0e93ff4..8b2517de108 100644 --- a/media/multipart/src/main/java/io/helidon/media/multipart/MimeParser.java +++ b/media/multipart/src/main/java/io/helidon/media/multipart/MimeParser.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2021 Oracle and/or its affiliates. + * Copyright (c) 2020, 2022 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -83,12 +83,14 @@ abstract static class ParserEvent { /** * Get the event type. + * * @return EVENT_TYPE */ abstract EventType type(); /** * Get this event as a {@link HeaderEvent}. + * * @return HeaderEvent */ HeaderEvent asHeaderEvent() { @@ -97,6 +99,7 @@ HeaderEvent asHeaderEvent() { /** * Get this event as a {@link BodyEvent}. + * * @return HeaderEvent */ BodyEvent asBodyEvent() { @@ -229,6 +232,7 @@ static final class ParsingException extends RuntimeException { /** * Create a new exception with the specified message. + * * @param message exception message */ private ParsingException(String message) { @@ -237,6 +241,7 @@ private ParsingException(String message) { /** * Create a new exception with the specified cause. + * * @param cause exception cause */ private ParsingException(Throwable cause) { @@ -344,8 +349,8 @@ private enum STATE { * Push new data to the parsing buffer. * * @param data new data add to the parsing buffer - * @throws ParsingException if the parser state is not consistent * @return buffer id + * @throws ParsingException if the parser state is not consistent */ int offer(ByteBuffer data) throws ParsingException { if (closed) { @@ -358,6 +363,9 @@ int offer(ByteBuffer data) throws ParsingException { break; case DATA_REQUIRED: // resume the previous state + if (LOGGER.isLoggable(Level.FINER)) { + LOGGER.log(Level.FINER, "Resume state. resumeState={1}", resumeState); + } state = resumeState; resumeState = null; id = buf.offer(data, position); @@ -409,6 +417,7 @@ void cleanup() { /** * Advances parsing. + * * @throws ParsingException if an error occurs during parsing */ Iterator parseIterator() { @@ -574,9 +583,13 @@ private List readBody() { position = bufLen - (bl + 1); return buf.slice(bodyBegin, position); } - // remaining data can be an complete boundary, force it to be + // remaining data can be a complete boundary, force it to be // processed during next iteration return Collections.emptyList(); + } else if (bndStart + bl + 1 >= bufLen) { + // found a boundary at the very end of the buffer + // it may be a "closing" boundary, require more data + return Collections.emptyList(); } // Found boundary. @@ -724,7 +737,7 @@ private String readHeaderLine() { } } position = offset + hdrLen + lwsp; - if (hdrLen == 0){ + if (hdrLen == 0) { return ""; } return new String(buf.getBytes(offset, hdrLen), HEADER_ENCODING); @@ -733,7 +746,7 @@ private String readHeaderLine() { /** * Boyer-Moore search method. * Copied from {@link java.util.regex.Pattern} - * + *

* Pre calculates arrays needed to generate the bad character shift and the * good suffix shift. Only the last seven bits are used to see if chars * match; This keeps the tables small and covers the heavily used ASCII @@ -809,6 +822,7 @@ private int match() { /** * Get the bytes representation of a string. + * * @param str string to convert * @return byte[] */ diff --git a/media/multipart/src/test/java/io/helidon/media/multipart/MimeParserTest.java b/media/multipart/src/test/java/io/helidon/media/multipart/MimeParserTest.java index 564a9acf689..64ccb4eb7f6 100644 --- a/media/multipart/src/test/java/io/helidon/media/multipart/MimeParserTest.java +++ b/media/multipart/src/test/java/io/helidon/media/multipart/MimeParserTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2020, 2022 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,11 +16,11 @@ package io.helidon.media.multipart; import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; import java.util.List; -import java.util.ArrayList; -import java.util.HashMap; import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -32,8 +32,8 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.hasItems; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; @@ -59,7 +59,7 @@ public static void after() { // TODO test mixed with nested boundaries @Test - public void testSkipPreambule() { + public void testSkipPreamble() { String boundary = "boundary"; final byte[] chunk1 = ("--" + boundary + " \t \t \t " @@ -79,7 +79,7 @@ public void testSkipPreambule() { } @Test - public void testNoPreambule() { + public void testNoPreamble() { String boundary = "boundary"; final byte[] chunk1 = ("--" + boundary + "Content-Id: part1\n" @@ -431,6 +431,46 @@ public void testClosingBoundaryAcrossChunks() { assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1"))); } + @Test + public void testIncompleteClosingBoundary() { + String boundary = "boundary"; + final byte[] chunk1 = ("--" + boundary + "\n" + + "Content-Id: part1\n" + + "\n" + + "this-is-the-body-of-part1\n" + + "--" + boundary + "-").getBytes(); + final byte[] chunk2 = "-".getBytes(); + + List parts = parse(boundary, List.of(chunk1, chunk2)).parts; + assertThat(parts.size(), is(equalTo(1))); + + MimePart part1 = parts.get(0); + assertThat(part1.headers.size(), is(equalTo(1))); + assertThat(part1.headers.get("Content-Id"), hasItems("part1")); + assertThat(part1.content, is(notNullValue())); + assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1"))); + } + + @Test + public void testAmbiguousClosingBoundary() { + String boundary = "boundary"; + final byte[] chunk1 = ("--" + boundary + "\n" + + "Content-Id: part1\n" + + "\n" + + "this-is-the-body-of-part1\n" + + "--" + boundary).getBytes(); + final byte[] chunk2 = "--".getBytes(); + + List parts = parse(boundary, List.of(chunk1, chunk2)).parts; + assertThat(parts.size(), is(equalTo(1))); + + MimePart part1 = parts.get(0); + assertThat(part1.headers.size(), is(equalTo(1))); + assertThat(part1.headers.get("Content-Id"), hasItems("part1")); + assertThat(part1.content, is(notNullValue())); + assertThat(new String(part1.content), is(equalTo("this-is-the-body-of-part1"))); + } + @Test public void testPreamble() { String boundary = "boundary"; @@ -688,12 +728,8 @@ static ParserResult parse(String boundary, List data) { assertThat(name, notNullValue()); assertThat(name.length(), not(equalTo(0))); assertThat(value, notNullValue()); - List values = partHeaders.get(name); - if (values == null) { - values = new ArrayList<>(); - partHeaders.put(name, values); - } - values.add(value); + partHeaders.computeIfAbsent(name, k -> new ArrayList<>()) + .add(value); break; case BODY: for (VirtualBuffer.BufferEntry content : event.asBodyEvent().body()) {