tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rémy Maucherat <r...@apache.org>
Subject Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)
Date Tue, 03 Jul 2018 08:55:06 GMT
On Tue, Jul 3, 2018 at 10:31 AM Piotr Joński <p.jonski@pojo.pl> wrote:

> 1. Originally I have sent multipart/mixed request, but tomcat does not
> support it at all! Tomcat supports only form-data, however in RFC, the very
> first example is about mixed:
> https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html


Quick clarification: the Servlet API only supports multipart/form-data, so
this is normal.


>
> 2. I had to add Content-disposition: form-data; name="some-name";
> filename="some-file" because without that it was failing due to missing
> field name and filename.
>

If you have "Content-disposition: form-data", you still need a name but the
filename is optional. In that case, the part is also in the request
parameters.

Rémy


>
> I hope you manage to reproduce it. The simplest way is to use srping boot
> app with tomcat, which is chosen by default and send that request. Good
> luck!
>
> Thanks !
>
> On 3 July 2018 at 10:24, Rémy Maucherat <remm@apache.org> wrote:
>
> > On Mon, Jul 2, 2018 at 5:14 PM Piotr Joński <p.jonski@pojo.pl> wrote:
> >
> > > Hi, of course I use it together with multipart request.
> > > I have spring boot 2 + zuul on tomcat 8.5.31. And I cannot proxy
> traffic
> > > with multipart request due to that error.
> > > I know that available return the right number of bytes but later you
> have
> > > method makeAvailable() which tries to read more than allowed! Some
> greedy
> > > developer wrote that :)
> > > Please check unit tests which I added. The should explain you
> everything.
> > >
> >
> > Hum, ok, but there's no multipart boundary in your test. Do you have an
> > example of multipart content that fails to be processed correctly ?
> > makeAvailable will never try to read beyond the boundary position.
> > Personally, I don't see it as a problem if there are exceptions trying to
> > process non multipart content, but this sort of cleaner error handling is
> > often added.
> >
> >
> > >
> > > Also here is example issue:
> > >
> > > https://stackoverflow.com/questions/3263809/apache-
> > commons-file-upload-stream-ended-unexpectedly
> > > I saw a lot of them -- all unresolved.
> > >
> >
> > Maybe, but this one is about Tomcat 6, quite a while ago.
> >
> > Fileupload is a separate component. Of course, we do fix and update it as
> > needed.
> >
> > Rémy
> >
> >
> > >
> > > On 2 July 2018 at 16:58, Rémy Maucherat <remm@apache.org> wrote:
> > >
> > > > On Mon, Jul 2, 2018 at 4:35 PM Piotr Joński <p.jonski@pojo.pl>
> wrote:
> > > >
> > > > > Java: openjdk version "1.8.0_163"
> > > > > OpenJDK Runtime Environment (Zulu 8.28.0.1-linux64) (build
> > > 1.8.0_163-b01)
> > > > > OpenJDK 64-Bit Server VM (Zulu 8.28.0.1-linux64) (build 25.163-b01,
> > > mixed
> > > > > mode)
> > > > >
> > > > > OS: Ubuntu 18.04 Linux local 4.15.0-23-generic #25-Ubuntu SMP Wed
> May
> > > 23
> > > > > 18:02:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
> > > > >
> > > > >
> > > > > The problem is that
> > > > >
> > > > > org.apache.tomcat.util.http.fileupload.MultipartStream.
> > > > ItemInputStream#read(byte[],
> > > > > int, int) method does not update pos field after reading from
> buffer
> > /
> > > > > stream.
> > > > >
> > > >
> > > > pos is set to the next separator which (IMO) should work fine since
> > > > available returns the right amount of bytes which are allowed to be
> > read.
> > > > Doesn't this work for you ? This is not a generic utility class, it's
> > > > supposed to be used with multipart content, is it at least what you
> are
> > > > doing ?
> > > >
> > > > Rémy
> > > >
> > > >
> > > > >
> > > > > Unfortunately I cannot provide full example as this is private
> > project.
> > > > >
> > > > > Here are sample unit tests. First reproduces the error and second
> use
> > > > > reflection to set proper field value to simulate proper behaviour:
> > > > >
> > > > > package org.apache.tomcat.util.http.fileupload;
> > > > >
> > > > > import org.junit.jupiter.api.Test;
> > > > >
> > > > > import java.io.ByteArrayInputStream;
> > > > > import java.io.IOException;
> > > > > import java.lang.reflect.Field;
> > > > >
> > > > > import static org.assertj.core.api.Assertions.assertThat;
> > > > >
> > > > > class ItemInputStreamTest {
> > > > >
> > > > >     @Test
> > > > >     void Should_Read_Bytes_But_Throws_Exception() throws
> > IOException {
> > > > >         // given
> > > > >         byte[] bytes = new byte[]{1, 2, 3};
> > > > >         final ByteArrayInputStream inputStream = new
> > > > > ByteArrayInputStream(bytes);
> > > > >         final MultipartStream.ProgressNotifier progressNotifier =
> > new
> > > > > MultipartStream.ProgressNotifier(null, 1111);
> > > > >         final MultipartStream multipartStream = new
> > > > > MultipartStream(inputStream,
> > > > >
> > > >  bytes,
> > > > >
> > > > > progressNotifier);
> > > > >         MultipartStream.ItemInputStream itemInputStream =
> > > > > multipartStream.new ItemInputStream();
> > > > >
> > > > >         // when
> > > > >         byte[] buffer = new byte[8196];
> > > > >         int result = itemInputStream.read(buffer, 0, 8196);
> > > > >
> > > > >         // then
> > > > >         assertThat(result).isEqualTo(3);
> > > > >     }
> > > > >
> > > > >     @Test
> > > > >     void Should_Read_Bytes_Fixed() throws IOException,
> > > > > NoSuchFieldException, IllegalAccessException {
> > > > >         // given
> > > > >         byte[] bytes = new byte[]{1, 2, 3};
> > > > >         final ByteArrayInputStream inputStream = new
> > > > > ByteArrayInputStream(bytes);
> > > > >         final MultipartStream.ProgressNotifier progressNotifier =
> > new
> > > > > MultipartStream.ProgressNotifier(null, 1111);
> > > > >         final MultipartStream multipartStream = new
> > > > > MultipartStream(inputStream,
> > > > >
> > > >  bytes,
> > > > >
> > > > > progressNotifier);
> > > > >         MultipartStream.ItemInputStream itemInputStream =
> > > > > multipartStream.new ItemInputStream();
> > > > >
> > > > >         Field pos = itemInputStream.getClass()
> > > > >                                    .getDeclaredField("pos");
> > > > >         pos.setAccessible(true);
> > > > >         pos.set(itemInputStream, 3);
> > > > >
> > > > >         // when
> > > > >         byte[] buffer = new byte[8196];
> > > > >         int result = itemInputStream.read(buffer, 0, 8196);
> > > > >
> > > > >         // then
> > > > >         assertThat(result).isEqualTo(3);
> > > > >     }
> > > > > }
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message