Hi Bongjae,
thank you for the patch!
It looks good, just something wanted to check with you:
1) do we really have to update existing NIOInputStreamImpl? IMO
inputBuffer should never be null
2) we can make CompletionHandler to be a part of MultipartContext, so it
will be accessible everywhere. We can make CompletionHandler attribute
protected or package private and don't expose it outside.
What do you think?
Thanks.
WBR,
Alexey.
On 13.02.14 01:14, Bongjae Chang wrote:
> Hi Alexey,
>
> I attached a proposed patch based on master branch about this issue.
>
> When I reviewed sources, MultipartEntryNIOInputStream should hold user¹s
> completion handler in order to call CompletionHandler#failed because we
> can't control the user¹s ReadHandler without wrapping it.
>
> Could you review it?
>
> Thanks!
>
> Regards,
> Bongjae Chang
>
>
>
>
>
> On 2/13/14, 12:04 PM, "Bongjae Chang" <bongjae.chang_at_gmail.com> wrote:
>
>> Hi Alexey,
>>
>> Ok. I will investigate it.
>>
>> Thanks.
>>
>> Regards,
>> Bongjae Chang
>>
>>
>>
>>
>>
>> On 2/13/14, 10:52 AM, "Oleksiy Stashok" <oleksiy.stashok_at_oracle.com>
>> wrote:
>>
>>> Hi Bongjae,
>>>
>>> I think it's a bug, CompletionHandler#failed has to be called.
>>> Do you have time to submit a fix?
>>>
>>> Thanks.
>>>
>>> WBR,
>>> Alexey.
>>>
>>>
>>> On 11.02.14 18:44, Bongjae Chang wrote:
>>>> Hi,
>>>>
>>>> When I tested multipart with http-multipart-samples sources, I met
>>>> some strange behavior.
>>>> I expected a failed notification of CompletionHandler if an error was
>>>> occurred in processing multipart, but there was no signal in some case.
>>>>
>>>> As you know, a user should implement two interfaces in order to call
>>>> MultipartScanner.scan() successfully.
>>>> 1. MultipartEntryHandler
>>>> 2. ReadHandler in MultipartEntryHandler
>>>>
>>>> If ReadHandler throws an exception in processing some works,
>>>> ReadHandler#onError() will be called. But user¹s
>>>> CompletionHandler#failed() was not called.
>>>>
>>>> You can reproduce this with samples easily like this.
>>>>
>>>> In UploaderHttpHandler.java
>>>> ‹
>>>> private void readAndSaveAvail() throws IOException {
>>>> while(inputStream.isReady()) {
>>>> final int readBytes = inputStream.read(buf);
>>>>
>>>> // limit max size for uploading a file.
>>>> final long currentBytes = uploadedBytesCounter.addAndGet(readBytes);
>>>> if (currentBytes >= 1024 * 1000) {
>>>> // throw exception for stopping upload
>>>> throw new IOException(³maximum size exceeded²);
>>>> }
>>>> ...
>>>> }
>>>> }
>>>>
>>>> I would like to know this behavior is intended.
>>>> (In other words, I would like to know if user should call
>>>> CompletionHandler#failed directly in ReadHandler#onError.)
>>>>
>>>> Thanks!
>>>>
>>>> Regards,
>>>> Bongjae Chang
>>>>