On 11/26/12 4:22 PM, Shing Wai Chan wrote:
> On 11/24/12 9:13 AM, Mark Thomas wrote:
>> Please find below comments generated while working on implementing the
>> HTTP Upgrade alongside JSR-356.
>>
>> ProtocolHandler
>> - Could we rename this UpgradeHandler? ProtocolHandler clashes with
>> something Tomcat uses internally. I'd also be happy with (in fact I
>> think I prefer) HttpUpgradeHandler. The less generic name we use, the
>> less chances of a conflict and the more obvious it is to our users
>> what the class does.
> +1
>>
>> ProtocolHandler / WebConnection
>> - There needs to be a mechanism for the new protocol to specify the
>> timeout for the socket. The new protocol may not want to continue to
>> use the same settings as HTTP
>>
>> ServletInputStream
>> - Given it is illegal to call read() when !isReady(), what happens if
>> one does call read()? IllegalStateException?
> If isReady() is, in fact, false, then read() will be blocking rather
> than throwing ISE in this case.
No we won't block. In the spec today we say that it is illegal but don't
say what containers are required to do. I think throwing an ISE is
probably what we should require containers to do.
>> - Is it legal to call isReady() before setReadListener() has been
>> called? My view is no, and that an IllegalStateException should be
>> thrown.
> It is legal.
Again - isReady has no way to notify anyone if a setReadListener call
isn't made. So we should throw an ISE here too.
>> - Should an NPE be thrown for setReadListener(null)?
> +1
>>
>> ServletOutputStream
>> - Given it is illegal to call write() when !canWrite(), what happens if
>> one does call write()? IllegalStateException?
> If canWrite() is, in fact, false, then write() will be blocking rather
> than throwing ISE in this case.
ISE here too.
>> - Is it legal to call canWrite() before setWriteListener() has been
>> called? My view is no, and that an IllegalStateException should be
>> thrown.
> It is legal.
ISE here too.
>> - Should an NPE be thrown for setWriteListener(null)?
> +1
>>
>> Cheers,
>>
>> Mark
>