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.
> - 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.
> - 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.
> - 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.
> - Should an NPE be thrown for setWriteListener(null)?
+1
>
> Cheers,
>
> Mark