Martin Cooper wrote:
> Let's hope. ;-) Please add youself to the list of developers in the
> project.xml file.
Done.
> This looks interesting. However, given the non-trivial nature of the
> changes, and the "stylistic" effect on the way FileUpload works, I
> wonder if
> this wouldn't be better incorporated into the FileUpload 2 design, rather
> than being retrofitted into FileUpload 1.x.
I am open for both. In either way, we can start in a branch. That leaves
everything open, doesn't it? I have one concern, though: Please let's try to
have a at least a beta release within short time, say three months?
> Well, I guess I don't agree here. If FileItem extended SimFileItem, then I
> could pass a FileItem to methods that accept a SlimFileItem. At that point,
> that SlimFileItem may or may not throw an exception if getInputStream is
> called more than once. That's bad API behaviour.
Personally I have no problem with my suggestion: If a method would accept a
StreamingFileItem, then that method should not expect, that getInputStream()
cannot be used more than once, so that's okay.
However, let's say that FileItem doesn't extend StreamingFileItem. Fine with me.
> SlimFileItem is best implemented as an inner class of the MultipartStream.
>
>
> Hmm. Not if you expect to expose SlimFileItem as part of the public API,
> which you are suggesting with the getItemIterator method below.
That's something I do not understand. The StreamingFileItem is an interface,
which is exposed. What's the problem, if the implementation is an inner class?
> Multipartstream isn't (supposed to be) part of the public API, so exposing
> an inner class of it through the public API is a no-no.
That's good to know. I get you right, that this means we may change that
classes API, may we?
> No. A FileUploadException isn't always related to IO, so extending
> IOException would simply be wrong.
Ok.
Attached you find a proposed patch, that takes a first step: The
MultipartStream receives an inner class ItemInputStream. This inner class is
used by readBodyData. (Obviously, the same InputStream will later be
returned by the StreamingFileItem.)
Jochen
Index: /home/jwi/workspace/commons-fileupload/src/java/org/apache/commons/fileupload/MultipartStream.java
===================================================================
--- /home/jwi/workspace/commons-fileupload/src/java/org/apache/commons/fileupload/MultipartStream.java (revision 413075)
+++ /home/jwi/workspace/commons-fileupload/src/java/org/apache/commons/fileupload/MultipartStream.java (working copy)
@@ -496,64 +496,22 @@
public int readBodyData(OutputStream output)
throws MalformedStreamException,
IOException {
- boolean done = false;
- int pad;
- int pos;
- int bytesRead;
- int total = 0;
- while (!done) {
- // Is boundary token present somewere in the buffer?
- pos = findSeparator();
- if (pos != -1) {
- // Write the rest of the data before the boundary.
+ final ItemInputStream istream = new ItemInputStream();
+ final byte[] bytes = new byte[8192];
+ for (;;) {
+ int res = istream.read(bytes);
+ if (res == -1) {
if (output != null) {
- output.write(buffer, head, pos - head);
+ output.flush();
}
- total += pos - head;
- head = pos;
- done = true;
- } else {
- // Determine how much data should be kept in the
- // buffer.
- if (tail - head > keepRegion) {
- pad = keepRegion;
- } else {
- pad = tail - head;
- }
- // Write out the data belonging to the body-data.
+ return (int) istream.getBytesRead();
+ }
+ if (res > 0 && output != null) {
if (output != null) {
- output.write(buffer, head, tail - head - pad);
- }
-
- // Move the data to the beginning of the buffer.
- total += tail - head - pad;
- System.arraycopy(buffer, tail - pad, buffer, 0, pad);
-
- // Refill buffer with new data.
- head = 0;
- bytesRead = input.read(buffer, pad, bufSize - pad);
-
- // [pprrrrrrr]
- if (bytesRead != -1) {
- tail = pad + bytesRead;
- } else {
- // The last pad amount is left in the buffer.
- // Boundary can't be in there so write out the
- // data you have and signal an error condition.
- if (output != null) {
- output.write(buffer, 0, pad);
- output.flush();
- }
- total += pad;
- throw new MalformedStreamException(
- "Stream ended unexpectedly");
+ output.write(bytes, 0, res);
}
}
}
- if (output != null) {
- output.flush();
- }
- return total;
}
@@ -748,6 +706,122 @@
}
}
+ /**
+ * An [EMAIL PROTECTED] InputStream} for reading an items contents.
+ */
+ public class ItemInputStream extends InputStream {
+ private long total;
+ private int pad, pos;
+
+ ItemInputStream() {
+ findSeparator();
+ }
+
+ private void findSeparator() {
+ pos = MultipartStream.this.findSeparator();
+ if (pos == -1) {
+ if (tail - head > keepRegion) {
+ pad = keepRegion;
+ } else {
+ pad = tail - head;
+ }
+ }
+ }
+
+ /** Returns the number of bytes, which have been read
+ * by the stream.
+ */
+ public long getBytesRead() {
+ return total;
+ }
+
+ public int available() throws IOException {
+ if (pos == -1) {
+ return tail - head - pad;
+ } else {
+ return pos - head;
+ }
+ }
+
+ public int read() throws IOException {
+ if (available() == 0) {
+ if (makeAvailable() == 0) {
+ return -1;
+ }
+ }
+ ++total;
+ int b = buffer[head++];
+ return b >= 0 ? b : b + 256;
+ }
+
+ public int read(byte[] b, int off, int len) throws IOException {
+ if (len == 0) {
+ return 0;
+ }
+ int res = available();
+ if (res == 0) {
+ res = makeAvailable();
+ if (res == 0) {
+ return -1;
+ }
+ }
+ res = Math.min(res, len);
+ System.arraycopy(buffer, head, b, off, res);
+ head += res;
+ total += res;
+ return res;
+ }
+
+ public void close() throws IOException {
+ for (;;) {
+ int av = available();
+ if (av == 0) {
+ av = makeAvailable();
+ if (av == 0) {
+ break;
+ }
+ }
+ skip(av);
+ }
+ }
+
+ public long skip(long bytes) throws IOException {
+ int av = available();
+ if (av == 0) {
+ av = makeAvailable();
+ if (av == 0) {
+ return 0;
+ }
+ }
+ long res = Math.min(av, bytes);
+ head += res;
+ return res;
+ }
+
+ private int makeAvailable() throws IOException {
+ if (pos != -1) {
+ return 0;
+ }
+
+ // Move the data to the beginning of the buffer.
+ total += tail - head - pad;
+ System.arraycopy(buffer, tail - pad, buffer, 0, pad);
+
+ // Refill buffer with new data.
+ head = 0;
+ int bytesRead = input.read(buffer, pad, bufSize - pad);
+ if (bytesRead == -1) {
+ // The last pad amount is left in the buffer.
+ // Boundary can't be in there so signal an error
+ // condition.
+ throw new MalformedStreamException(
+ "Stream ended unexpectedly");
+ }
+ tail = pad + bytesRead;
+ findSeparator();
+ return available();
+ }
+ }
// ------------------------------------------------------ Debugging methods
Index: /home/jwi/workspace/commons-fileupload/src/test/org/apache/commons/fileupload/SizesTest.java
===================================================================
--- /home/jwi/workspace/commons-fileupload/src/test/org/apache/commons/fileupload/SizesTest.java (revision 0)
+++ /home/jwi/workspace/commons-fileupload/src/test/org/apache/commons/fileupload/SizesTest.java (revision 0)
@@ -0,0 +1,81 @@
+/*
+ * Copyright 2001-2004 The Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.fileupload;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import junit.framework.TestCase;
+
+
+/**
+ * Unit test for items with varying sizes.
+ */
+public class SizesTest extends TestCase
+{
+ public void testFileUpload()
+ throws IOException, FileUploadException
+ {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ int add = 16;
+ int num = 0;
+ for (int i = 0; i < 16384; i += add) {
+ if (++add == 32) {
+ add = 16;
+ }
+ String header = "-----1234\r\n"
+ + "Content-Disposition: form-data; name=\"field" + (num++) + "\"\r\n"
+ + "\r\n";
+ baos.write(header.getBytes("US-ASCII"));
+ for (int j = 0; j < i; j++) {
+ baos.write((byte) j);
+ }
+ baos.write("\r\n".getBytes("US-ASCII"));
+ }
+ baos.write("-----1234--\r\n".getBytes("US-ASCII"));
+
+ List fileItems = parseUpload(baos.toByteArray());
+ Iterator fileIter = fileItems.iterator();
+ add = 16;
+ num = 0;
+ for (int i = 0; i < 16384; i += add) {
+ if (++add == 32) {
+ add = 16;
+ }
+ FileItem item = (FileItem) fileIter.next();
+ assertEquals("field" + (num++), item.getFieldName());
+ byte[] bytes = item.get();
+ assertEquals(i, bytes.length);
+ for (int j = 0; j < i; j++) {
+ assertEquals((byte) j, bytes[j]);
+ }
+ }
+ assertTrue(!fileIter.hasNext());
+ }
+
+ private List parseUpload(byte[] bytes) throws FileUploadException {
+ String contentType = "multipart/form-data; boundary=---1234";
+
+ FileUploadBase upload = new DiskFileUpload();
+ HttpServletRequest request = new MockHttpServletRequest(bytes, contentType);
+
+ List fileItems = upload.parseRequest(request);
+ return fileItems;
+ }
+
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
