From 9410a8d81c318b26fde9aa376186cb6a33881402 Mon Sep 17 00:00:00 2001 From: Thomas Becker Date: Wed, 20 Jun 2012 17:16:33 +0200 Subject: [PATCH] 383116: fix truncated filenames for multipart file uploads containing spaces in filename --- .../jetty/util/MultiPartInputStream.java | 10 +- .../jetty/util/MultiPartInputStreamTest.java | 91 ++++++++++++------- 2 files changed, 62 insertions(+), 39 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStream.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStream.java index 8de5efbcdad..d483ea82225 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStream.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStream.java @@ -371,7 +371,7 @@ public class MultiPartInputStream if (!_tmpDir.exists()) _tmpDir.mkdirs(); - String boundary="--"+QuotedStringTokenizer.unquote(value(_contentType.substring(_contentType.indexOf("boundary="))).trim()); + String boundary="--"+QuotedStringTokenizer.unquote(value(_contentType.substring(_contentType.indexOf("boundary=")), true).trim()); byte[] byteBoundary=(boundary+"--").getBytes(StringUtil.__ISO_8859_1); // Get first boundary @@ -440,9 +440,9 @@ public class MultiPartInputStream if(t.startsWith("form-data")) form_data=true; else if(tl.startsWith("name=")) - name=value(t); + name=value(t, true); else if(tl.startsWith("filename=")) - filename=value(t); + filename=value(t, false); } // Check disposition @@ -588,7 +588,7 @@ public class MultiPartInputStream /* ------------------------------------------------------------ */ - private String value(String nameEqualsValue) + private String value(String nameEqualsValue, boolean splitAfterSpace) { String value=nameEqualsValue.substring(nameEqualsValue.indexOf('=')+1).trim(); int i=value.indexOf(';'); @@ -598,7 +598,7 @@ public class MultiPartInputStream { value=value.substring(1,value.indexOf('"',1)); } - else + else if (splitAfterSpace) { i=value.indexOf(' '); if(i>0) diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/MultiPartInputStreamTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/MultiPartInputStreamTest.java index b4b0b8f611d..38e8c49d500 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/MultiPartInputStreamTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/MultiPartInputStreamTest.java @@ -13,9 +13,16 @@ package org.eclipse.jetty.util; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.IOException; import java.io.InputStream; import java.util.Collection; @@ -25,6 +32,8 @@ import javax.servlet.http.Part; import junit.framework.TestCase; +import org.eclipse.jetty.util.MultiPartInputStream.MultiPart; + /** * MultiPartInputStreamTest * @@ -32,19 +41,9 @@ import junit.framework.TestCase; */ public class MultiPartInputStreamTest extends TestCase { + private static final String FILENAME = "stuff.txt"; protected String _contentType = "multipart/form-data, boundary=AaB03x"; - protected String _multi = - "--AaB03x\r\n"+ - "content-disposition: form-data; name=\"field1\"\r\n"+ - "\r\n"+ - "Joe Blow\r\n"+ - "--AaB03x\r\n"+ - "content-disposition: form-data; name=\"stuff\"; filename=\"stuff.txt\"\r\n"+ - "Content-Type: text/plain\r\n"+ - "\r\n"+ - "000000000000000000000000000000000000000000000000000\r\n"+ - "--AaB03x--\r\n"; - + protected String _multi = createMultipartRequestString(FILENAME); protected String _dirname = System.getProperty("java.io.tmpdir")+File.separator+"myfiles-"+System.currentTimeMillis(); @@ -114,18 +113,28 @@ public class MultiPartInputStreamTest extends TestCase public void testMulti () throws Exception + { + testMulti(FILENAME); + } + + public void testMultiWithSpaceInFilename() throws Exception + { + testMulti("stuff with spaces.txt"); + } + + private void testMulti(String filename) throws IOException, ServletException { MultipartConfigElement config = new MultipartConfigElement(_dirname, 1024, 3072, 50); - MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(_multi.getBytes()), - _contentType, - config, - new File(_dirname)); + MultiPartInputStream mpis = new MultiPartInputStream(new ByteArrayInputStream(createMultipartRequestString(filename).getBytes()), + _contentType, + config, + new File(_dirname)); Collection parts = mpis.getParts(); - assertEquals(2, parts.size()); + assertThat(parts.size(), is(2)); Part field1 = mpis.getPart("field1"); - assertNotNull(field1); - assertEquals("field1", field1.getName()); + assertThat(field1,notNullValue()); + assertThat(field1.getName(),is("field1")); InputStream is = field1.getInputStream(); ByteArrayOutputStream os = new ByteArrayOutputStream(); IO.copy(is, os); @@ -143,23 +152,23 @@ public class MultiPartInputStreamTest extends TestCase assertFalse(f.exists()); //should have been renamed field1.delete(); //file should be deleted assertFalse(f2.exists()); - - Part stuff = mpis.getPart("stuff"); - assertEquals("text/plain", stuff.getContentType()); - assertEquals("text/plain", stuff.getHeader("Content-Type")); - assertEquals(1, stuff.getHeaders("content-type").size()); - assertEquals("form-data; name=\"stuff\"; filename=\"stuff.txt\"", stuff.getHeader("content-disposition")); - assertEquals(2, stuff.getHeaderNames().size()); - assertEquals(51, stuff.getSize()); + MultiPart stuff = (MultiPart)mpis.getPart("stuff"); + assertThat(stuff.getContentDispositionFilename(), is(filename)); + assertThat(stuff.getContentType(),is("text/plain")); + assertThat(stuff.getHeader("Content-Type"),is("text/plain")); + assertThat(stuff.getHeaders("content-type").size(),is(1)); + assertThat(stuff.getHeader("content-disposition"),is("form-data; name=\"stuff\"; filename=\"" + filename + "\"")); + assertThat(stuff.getHeaderNames().size(),is(2)); + assertThat(stuff.getSize(),is(51L)); f = ((MultiPartInputStream.MultiPart)stuff).getFile(); - assertNotNull(f); // longer than 100 bytes, should already be a file - assertNull(((MultiPartInputStream.MultiPart)stuff).getBytes()); //not in internal buffer any more - assertTrue(f.exists()); - assertNotSame("stuff.txt", f.getName()); - stuff.write("stuff.txt"); - f = new File(_dirname+File.separator+"stuff.txt"); - assertTrue(f.exists()); + assertThat(f,notNullValue()); // longer than 100 bytes, should already be a file + assertThat(((MultiPartInputStream.MultiPart)stuff).getBytes(),nullValue()); //not in internal buffer any more + assertThat(f.exists(),is(true)); + assertThat(f.getName(),is(not("stuff with space.txt"))); + stuff.write(filename); + f = new File(_dirname+File.separator+filename); + assertThat(f.exists(),is(true)); } public void testMultiSameNames () @@ -193,4 +202,18 @@ public class MultiPartInputStreamTest extends TestCase assertNotNull(p); assertEquals(5, p.getSize()); } + + private String createMultipartRequestString(String filename) + { + return "--AaB03x\r\n"+ + "content-disposition: form-data; name=\"field1\"\r\n"+ + "\r\n"+ + "Joe Blow\r\n"+ + "--AaB03x\r\n"+ + "content-disposition: form-data; name=\"stuff\"; filename=\"" + filename + "\"\r\n"+ + "Content-Type: text/plain\r\n"+ + "\r\n"+ + "000000000000000000000000000000000000000000000000000\r\n"+ + "--AaB03x--\r\n"; + } }