Thank you Michael for reviewing and sponsoring. -Jaikiran
On 26/08/19 5:02 PM, Michael McMahon wrote: > Jaikiran, > > The CSR was approved, and the change has been pushed. > Thanks for the contribution. > > 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 >>>>>
