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