From 93b8161afb6738e06c9872690dc0e97bc40e9f03 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 20 Mar 2018 11:40:55 +1100 Subject: [PATCH] Reworked MultipartParser with additional option to accept CR as CRLF. Signed-off-by: Lachlan Roberts --- .../http/MultiPartInputStreamParser.java | 21 +++-- .../eclipse/jetty/http/MultiPartParser.java | 80 +++++++++++++++---- .../jetty/http/MultiPartInputStreamTest.java | 5 +- .../jetty/http/MultiPartParserTest.java | 19 ++--- 4 files changed, 86 insertions(+), 39 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartInputStreamParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartInputStreamParser.java index bfa01590f23..857909f6bd4 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartInputStreamParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartInputStreamParser.java @@ -497,6 +497,7 @@ public class MultiPartInputStreamParser { if (_err != null) { + _err.addSuppressed(new Throwable()); if (_err instanceof IOException) throw (IOException)_err; if (_err instanceof IllegalStateException) @@ -551,7 +552,6 @@ public class MultiPartInputStreamParser } - // ============ MY CODE ============= // Handler handler = new Handler(); MultiPartParser parser = new MultiPartParser(handler,contentTypeBoundary); @@ -596,7 +596,12 @@ public class MultiPartInputStreamParser //check we read to the end of the message if(parser.getState() != MultiPartParser.State.END) { - _err = new IllegalStateException("Parser Not in End State: "+parser.getState()); + _err = new IOException("Incomplete Multipart"); + } + + if(LOG.isDebugEnabled()) + { + LOG.debug("Parsing Complete {} err={}",parser,_err); } } @@ -668,16 +673,8 @@ public class MultiPartInputStreamParser { return false; } - - //Have a new Part - System.out.println("-------------------------"); - System.out.println("Creating New Part"); - System.out.println("Name: "+name); - System.out.println("Filename: "+filename); - System.out.println("Headers:\n"+headers); - System.out.println("ContentType:\n"+contentType); - System.out.println("-------------------------"); - + + //create the new part _part = new MultiPart(name, filename); _part.setHeaders(headers); _part.setContentType(contentType); diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartParser.java index 847a807e6c6..851054ea789 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartParser.java @@ -152,6 +152,8 @@ public class MultiPartParser private final boolean DEBUG=LOG.isDebugEnabled(); private final Handler _handler; private final SearchPattern _delimiterSearch; + private final boolean _acceptCrAsLineTermination; + private String _fieldName; private String _fieldValue; @@ -159,6 +161,7 @@ public class MultiPartParser private FieldState _fieldState = FieldState.FIELD; private int _partialBoundary = 2; // No CRLF if no preamble private boolean _cr; + private byte _next; private ByteBuffer _patternBuffer; private final StringBuilder _string=new StringBuilder(); @@ -167,12 +170,25 @@ public class MultiPartParser /* ------------------------------------------------------------------------------- */ public MultiPartParser(Handler handler, String boundary) + { + this(handler,boundary,false); + } + + /* ------------------------------------------------------------------------------- */ + /** TODO complete + * + * @param handler + * @param boundary + * @param acceptCR + */ + public MultiPartParser(Handler handler, String boundary, boolean acceptCR) { _handler = handler; String delimiter = "\r\n--"+boundary; _patternBuffer = ByteBuffer.wrap(delimiter.getBytes(StandardCharsets.US_ASCII)); _delimiterSearch = SearchPattern.compile(_patternBuffer.array()); + _acceptCrAsLineTermination = acceptCR; } public void reset() @@ -254,9 +270,23 @@ public class MultiPartParser } /* ------------------------------------------------------------------------------- */ - private byte next(ByteBuffer buffer) + private boolean hasNextByte(ByteBuffer buffer) { - byte ch = buffer.get(); + return BufferUtil.hasContent(buffer) || _next!=0; + } + + /* ------------------------------------------------------------------------------- */ + private byte getNextByte(ByteBuffer buffer, boolean last) + { + + byte ch; + if (_next==0) + ch = buffer.get(); + else + { + ch = _next; + _next = 0; + } CharState s = __charState[0xff & ch]; switch(s) @@ -267,11 +297,18 @@ public class MultiPartParser case CR: if (_cr) - throw new BadMessageException("Bad EOL"); - + { + if (!_acceptCrAsLineTermination) + throw new BadMessageException("Bad EOL"); + // TODO log compliance violation + return (byte)'\n'; + } + _cr=true; if (buffer.hasRemaining()) - return next(buffer); + return getNextByte(buffer, last); + else if (_acceptCrAsLineTermination && last) + return '\n'; // Can return 0 here to indicate the need for more characters, // because a real 0 in the buffer would cause a BadMessage below @@ -279,7 +316,14 @@ public class MultiPartParser case LEGAL: if (_cr) - throw new BadMessageException("Bad EOL"); + { + if (!_acceptCrAsLineTermination) + throw new BadMessageException("Bad EOL"); + // TODO log compliance violation + _next = ch; + _cr = false; + return (byte)'\n'; + } return ch; case ILLEGAL: @@ -328,11 +372,11 @@ public class MultiPartParser case DELIMITER: case DELIMITER_PADDING: case DELIMITER_CLOSE: - parseDelimiter(buffer); + parseDelimiter(buffer, last); continue; case BODY_PART: - handle = parseMimePartHeaders(buffer); + handle = parseMimePartHeaders(buffer, last); break; case FIRST_OCTETS: @@ -410,11 +454,11 @@ public class MultiPartParser } /* ------------------------------------------------------------------------------- */ - private void parseDelimiter(ByteBuffer buffer) + private void parseDelimiter(ByteBuffer buffer, boolean last) { - while (__delimiterStates.contains(_state) && buffer.hasRemaining()) + while (__delimiterStates.contains(_state) && hasNextByte(buffer)) { - byte b=next(buffer); + byte b=getNextByte(buffer, last); if (b==0) return; @@ -454,13 +498,13 @@ public class MultiPartParser /* * Parse the message headers and return true if the handler has signaled for a return */ - protected boolean parseMimePartHeaders(ByteBuffer buffer) + protected boolean parseMimePartHeaders(ByteBuffer buffer, boolean last) { // Process headers - while (_state==State.BODY_PART && buffer.hasRemaining()) + while (_state==State.BODY_PART && hasNextByte(buffer)) { // process each character - byte b=next(buffer); + byte b=getNextByte(buffer, last); if (b==0) break; @@ -629,6 +673,14 @@ public class MultiPartParser protected boolean parseOctetContent(ByteBuffer buffer) { + + //handle the next content that was held because of \r as \r\n + if (_next!=0) + { + _handler.content(BufferUtil.toBuffer(new byte[] {_next}),false); + _next = 0; + } + //Starts With if (_partialBoundary>0) { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartInputStreamTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartInputStreamTest.java index e833e16d1cb..1ee35872e2b 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartInputStreamTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartInputStreamTest.java @@ -77,7 +77,9 @@ public class MultiPartInputStreamTest "Content-Disposition: form-data; name=\"fileup\"; filename=\"test.upload\"\r\n"+ "Content-Type: application/octet-stream\r\n\r\n"+ "How now brown cow."+ - "\r\n--" + boundary + "-\r\n\r\n"; + "\r\n--" + boundary + "-\r\n" + + "Content-Disposition: form-data; name=\"fileup\"; filename=\"test.upload\"\r\n" + + "\r\n"; MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 50); MultiPartInputStreamParser mpis = new MultiPartInputStreamParser(new ByteArrayInputStream(str.getBytes()), @@ -92,6 +94,7 @@ public class MultiPartInputStreamTest } catch (IOException e) { + System.err.println(e.getMessage()); assertTrue(e.getMessage().startsWith("Incomplete")); } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartParserTest.java index 58e573bcae2..4dfe141969c 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartParserTest.java @@ -450,7 +450,7 @@ public class MultiPartParserTest @Test - public void testEndState() { + public void testCrAsLineTermination() { TestHandler handler = new TestHandler() { @Override public boolean messageComplete(){ return true; } @@ -462,19 +462,14 @@ public class MultiPartParserTest return false; } }; - MultiPartParser parser = new MultiPartParser(handler,"AaB03x"); + MultiPartParser parser = new MultiPartParser(handler,"AaB03x",true); - ByteBuffer data = BufferUtil.toBuffer(" "+ - "--AaB03x\r\n"+ - "content-disposition: form-data; name=\"field1\"\r\n"+ - "\r\n"+ + ByteBuffer data = BufferUtil.toBuffer( + "--AaB03x\r"+ + "content-disposition: form-data; name=\"field1\"\r"+ + "\r"+ "Joe Blow\r\n"+ - "--AaB03x\r\n"+ - "content-disposition: form-data; name=\"stuff\"; filename=\"" + "foo.txt" + "\"\r\n"+ - "Content-Type: text/plain\r\n"+ - "\r\n"+"aaaa"+ - "bbbbb"+"\r\n" + - "--AaB03x--\r\n"); + "--AaB03x--\r"); /* Test Progression to END State */