Jaikiran
It's not necessary to update the webrev with the change I made.
I've put it into my copy of the patch already.
Thanks
Michael
On 22/08/2019, 17:19, Jaikiran Pai wrote:
On 22/08/19 8:17 PM, Michael McMahon wrote:
Getting back to this issue. I have filed a CSR at
https://bugs.openjdk.java.net/browse/JDK-8230044
Thank you Michael.
for the minor doc/spec change for this.
changing the @throws spec for ProxySelector::select to
|+ * @throws IllegalArgumentException if the argument is null or if
+ * the protocol or host cannot be determined from the provided
+ * {@code uri}
|||
I changed the suggested wording slightly as follows: 'from the
provided {@code uri}'
I'll include this minor change in a updated webrev and send the link
tomorrow.
-Jaikiran
Thanks,
Michael.
||
On 01/08/2019, 11:52, Michael McMahon wrote:
Hi Jaikiran,
This looks good to me. There are a number of other places where
ProxySelector::select
gets called which could benefit from the same change. Personally, I
think it would be okay
to deal with those in a followup issue, but maybe others have
another view on that.
I can create the CSR for this, but it could be next week before I
get around to that.
Thanks
Michael.
On 01/08/2019, 08:03, Jaikiran Pai wrote:
Hello Michael,
On 31/07/19 11:43 AM, Michael McMahon wrote:
...
I wonder if another solution is possible where the IAE is caught
at the appropriate place(s) and converted to an IOException
which is what users are expecting.
I can update the proposed patch to redo it to catch the IAE and
throw
the IOException at the relevant places. Would you also want me to
update
any javadoc of the ProxySelector to make it more clear of this
(so far)
implicit behaviour? Does that require a CSR?
I think it would be good to do that, and it would require a CSR.
The existing @throws statement could be expanded to make this
explicit
perhaps.
I have now redone the patch to allow the DefaultProxySelector to
continue to throw the IAE for missing host/protocol in the URI and
then
let it be caught in the relevant places and throw a IOException. I
have
also updated the javadoc of ProxySelector to explicitly state this
previously implicit behaviour. The 2 new testcases have been
updated to
test these changes appropriately.
The new webrev is at
http://cr.openjdk.java.net/~jpai/webrev/defaultproxyselector/2/webrev/.
Apart from the review and sponsoring I will also need help on
creating a
CSR for this one.
-Jaikiran
