You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Piotr Joński <p....@pojo.pl> on 2018/07/02 14:35:24 UTC

Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

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.

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);
    }
}

Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

Posted by Piotr Joński <p....@pojo.pl>.
BTW2 "makeAvailable will never try to read beyond the boundary position."
-- this is not true. My content of json is properly read into buffer. after
that it finish because it hit boundary. And the method makeAccessible()
tried to read more from that stream but it finished!
BTW3 those 2 tests that i send you -- the first one is failing because of
missing "bugfix", the second use reflection to set proper value, and it is
working.

On 3 July 2018 at 10:31, Piotr Joński <p....@pojo.pl> wrote:

> BTW instead of ******************* you have to use some real values. of
> course I have just obfuscated values and left headers because they can
> matter.
>
> On 3 July 2018 at 10:30, Piotr Joński <p....@pojo.pl> wrote:
>
>> Hi, thanks for reply!
>> Here is sample snippet that i use to reproduce it:
>> ------------------------------------------------------------
>> ----------------------------------------------------------
>> #!/usr/bin/env bash
>> echo;
>> echo;
>> echo;
>> echo;
>> echo;
>> echo;
>> curl 'http://localhost:8083/************' \
>>      -H 'Origin: ***************' \
>>      -H 'X-Client-Version: 6.5.2-rc0' \
>>      -H 'Authorization: ********************' \
>>      -H 'Content-Type: multipart/form-data;boundary="======boundary======"'
>> \
>>      -H 'Accept: application/json, text/plain, */*' \
>>      -H 'Referer: *****************' \
>>      -H 'X-Client-ID: *************' \
>>      -H 'X-Request-Id: 16613F22FBF46FA9BA441125219C2B13' \
>>      -H 'X-B3-TraceId: 16613f22fbf46fb9ba441125219c3b03' \
>>      -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36
>> (KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36' \
>>      -H 'DNT: 1' \
>>      --data-binary $'--======boundary======\nContent-disposition:
>> form-data; name="some-name"; filename="some-file"\r\nContent-Type:
>> application/http\r\nContent-ID: req0\r\n\r\nPUT /api/*****
>> HTTP/1.1\nContent-Type: application/json\n\n{********j
>> son-here********}\r\n\r\n--======boundary======--' \
>>      --compressed
>> echo;
>> echo;
>> echo;
>> echo;
>> echo;
>> echo;
>> ------------------------------------------------------------
>> ----------------------------------------------------------
>>
>> 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/Prot
>> ocols/rfc1341/7_2_Multipart.html
>> 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.
>>
>> 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 <re...@apache.org> wrote:
>>
>>> On Mon, Jul 2, 2018 at 5:14 PM Piotr Joński <p....@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-f
>>> ile-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 <re...@apache.org> wrote:
>>> >
>>> > > On Mon, Jul 2, 2018 at 4:35 PM Piotr Joński <p....@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);
>>> > > >     }
>>> > > > }
>>> > > >
>>> > >
>>> >
>>>
>>
>>
>

Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

Posted by Piotr Joński <p....@pojo.pl>.
BTW instead of ******************* you have to use some real values. of
course I have just obfuscated values and left headers because they can
matter.

On 3 July 2018 at 10:30, Piotr Joński <p....@pojo.pl> wrote:

> Hi, thanks for reply!
> Here is sample snippet that i use to reproduce it:
> ------------------------------------------------------------
> ----------------------------------------------------------
> #!/usr/bin/env bash
> echo;
> echo;
> echo;
> echo;
> echo;
> echo;
> curl 'http://localhost:8083/************' \
>      -H 'Origin: ***************' \
>      -H 'X-Client-Version: 6.5.2-rc0' \
>      -H 'Authorization: ********************' \
>      -H 'Content-Type: multipart/form-data;boundary="======boundary======"'
> \
>      -H 'Accept: application/json, text/plain, */*' \
>      -H 'Referer: *****************' \
>      -H 'X-Client-ID: *************' \
>      -H 'X-Request-Id: 16613F22FBF46FA9BA441125219C2B13' \
>      -H 'X-B3-TraceId: 16613f22fbf46fb9ba441125219c3b03' \
>      -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36
> (KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36' \
>      -H 'DNT: 1' \
>      --data-binary $'--======boundary======\nContent-disposition:
> form-data; name="some-name"; filename="some-file"\r\nContent-Type:
> application/http\r\nContent-ID: req0\r\n\r\nPUT /api/*****
> HTTP/1.1\nContent-Type: application/json\n\n{********
> json-here********}\r\n\r\n--======boundary======--' \
>      --compressed
> echo;
> echo;
> echo;
> echo;
> echo;
> echo;
> ------------------------------------------------------------
> ----------------------------------------------------------
>
> 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
> 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.
>
> 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 <re...@apache.org> wrote:
>
>> On Mon, Jul 2, 2018 at 5:14 PM Piotr Joński <p....@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 <re...@apache.org> wrote:
>> >
>> > > On Mon, Jul 2, 2018 at 4:35 PM Piotr Joński <p....@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);
>> > > >     }
>> > > > }
>> > > >
>> > >
>> >
>>
>
>

Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Piotr,

On 7/3/18 4:30 AM, Piotr Joński wrote:
> Hi, thanks for reply! Here is sample snippet that i use to
> reproduce it: 
> ----------------------------------------------------------- 
> ----------------------------------------------------------- 
> #!/usr/bin/env bash echo; echo; echo; echo; echo; echo; curl
> 'http://localhost:8083/************' \ -H 'Origin: ***************'
> \ -H 'X-Client-Version: 6.5.2-rc0' \ -H 'Authorization:
> ********************' \ -H 'Content-Type:
> multipart/form-data;boundary="======boundary======"' \ -H 'Accept:
> application/json, text/plain, */*' \ -H 'Referer:
> *****************' \ -H 'X-Client-ID: *************' \ -H
> 'X-Request-Id: 16613F22FBF46FA9BA441125219C2B13' \ -H
> 'X-B3-TraceId: 16613f22fbf46fb9ba441125219c3b03' \ -H 'User-Agent:
> Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like
> Gecko) Chrome/67.0.3396.99 Safari/537.36' \ -H 'DNT: 1' \ 
> --data-binary $'--======boundary======\nContent-disposition: 
> form-data; name="some-name"; filename="some-file"\r\nContent-Type: 
> application/http\r\nContent-ID: req0\r\n\r\nPUT /api/***** 
> HTTP/1.1\nContent-Type: 
> application/json\n\n{********json-here********}\r\n\r\n--======boundar
y======--'
>
> 
\
> --compressed echo; echo; echo; echo; echo; echo;

I think you need a trailing \r\n after that last boundary to close the
message. The BNF in the RFC doesn't seem to require the tailing CRLF,
but MIME is a very "line-oriented" protocol, so I wouldn't be
surprised if it needs a "complete line", which would include a
trailing CRLF.

- -chris
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAls7u3gACgkQHPApP6U8
pFhQiBAAg7BLjQQeWqcxc4OGoKAWrMYcvBXohEh7Mo9JLm1I1ky7u8Uj3wFoTzF1
5mdMQigGr+OOjhYbEzlT/HIr4jXarcznmcITY7f9e6DnmAol7hXXBHr8D7vUlR0w
T5szi6ejJ20yyyfwqg/s5wNOIJC06LDPEUHWpO/AG7+6yfZ61g8UZfxi5y259RmU
KVg3CdysBTDOrWlFoclnsXttD4JJG+Vr1B9RfrEXuQf0cko5u+ArSmXivmEUJ+RM
IhgY2dqJSC2cf+7PNV8n8kAqmoPWN8aoe7CWL7gsb4U1XHV1l0INAvzEXBeBCVSp
XLwX0/nEMXNG0DeKN5WQDCZE6D0d5a5Nt4NGZCmNuYXzDGf095FLjpPaAgKADJVL
7b8lFA8d7+58CMlKAeP5C6d/wNR2NMyF4COgwC1LcKiMGyLGRNGVsI7MnG91sGdO
jb2rVYkeQ7k/+KHk4PTzKZ1Osff9eXU29L3SDb9MuHYJUCp9fyVmlyHYdOZq5qxM
YcGb5IwaEdC1rwO3syZSan81nyJiqpG+QH8g7viTq1h9WByVBLrwwgIaWSzCXIQM
D/ibaI5PtMj4Gbq08pgJTZTzZuRxfgFOnmqmnQW3jFe6Y//BaEYoI6yYQZxdmIYE
a9dE7oqBLslAq/fNKjQCGm6ymZoqT+PlXgEzQ8KQQZlSLVw54MQ=
=6JFv
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

Posted by Rémy Maucherat <re...@apache.org>.
On Tue, Jul 3, 2018 at 10:31 AM Piotr Joński <p....@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 <re...@apache.org> wrote:
>
> > On Mon, Jul 2, 2018 at 5:14 PM Piotr Joński <p....@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 <re...@apache.org> wrote:
> > >
> > > > On Mon, Jul 2, 2018 at 4:35 PM Piotr Joński <p....@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);
> > > > >     }
> > > > > }
> > > > >
> > > >
> > >
> >
>

Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

Posted by Piotr Joński <p....@pojo.pl>.
Hi, thanks for reply!
Here is sample snippet that i use to reproduce it:
-----------------------------------------------------------
-----------------------------------------------------------
#!/usr/bin/env bash
echo;
echo;
echo;
echo;
echo;
echo;
curl 'http://localhost:8083/************' \
     -H 'Origin: ***************' \
     -H 'X-Client-Version: 6.5.2-rc0' \
     -H 'Authorization: ********************' \
     -H 'Content-Type: multipart/form-data;boundary="======boundary======"'
\
     -H 'Accept: application/json, text/plain, */*' \
     -H 'Referer: *****************' \
     -H 'X-Client-ID: *************' \
     -H 'X-Request-Id: 16613F22FBF46FA9BA441125219C2B13' \
     -H 'X-B3-TraceId: 16613f22fbf46fb9ba441125219c3b03' \
     -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36
(KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36' \
     -H 'DNT: 1' \
     --data-binary $'--======boundary======\nContent-disposition:
form-data; name="some-name"; filename="some-file"\r\nContent-Type:
application/http\r\nContent-ID: req0\r\n\r\nPUT /api/*****
HTTP/1.1\nContent-Type:
application/json\n\n{********json-here********}\r\n\r\n--======boundary======--'
\
     --compressed
echo;
echo;
echo;
echo;
echo;
echo;
-----------------------------------------------------------
-----------------------------------------------------------

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
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.

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 <re...@apache.org> wrote:

> On Mon, Jul 2, 2018 at 5:14 PM Piotr Joński <p....@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 <re...@apache.org> wrote:
> >
> > > On Mon, Jul 2, 2018 at 4:35 PM Piotr Joński <p....@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);
> > > >     }
> > > > }
> > > >
> > >
> >
>

Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

Posted by Rémy Maucherat <re...@apache.org>.
On Mon, Jul 2, 2018 at 5:14 PM Piotr Joński <p....@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 <re...@apache.org> wrote:
>
> > On Mon, Jul 2, 2018 at 4:35 PM Piotr Joński <p....@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);
> > >     }
> > > }
> > >
> >
>

Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

Posted by Piotr Joński <p....@pojo.pl>.
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.

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.

On 2 July 2018 at 16:58, Rémy Maucherat <re...@apache.org> wrote:

> On Mon, Jul 2, 2018 at 4:35 PM Piotr Joński <p....@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);
> >     }
> > }
> >
>

Re: Bug in org.apache.tomcat.util.http.fileupload.MultipartStream.ItemInputStream#read(byte[], int, int)

Posted by Rémy Maucherat <re...@apache.org>.
On Mon, Jul 2, 2018 at 4:35 PM Piotr Joński <p....@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);
>     }
> }
>