RFR 6563286 6797318 8177648 - Undeclared IAE thrown from HttpURLConnection.connect for some URLs

[Jaikiran Pai]

Hello Michael,

On 01/08/19 4:22 PM, Michael McMahon wrote:
> Hi Jaikiran,
>
> This looks good to me. 

Thank you.


> 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.

Like you, I too thought we can address this in different subsequent
commit(s). Keeping aside references of this API in the test cases, I can
see 3 more references to this API spread across the java.net,
jdk.internal.net.http and jdk.internal.net.http.websocket packages. If
others feel these need to be updated in this round itself, I can submit
an updated webrev.


> I can create the CSR for this, but it could be next week before I get
> around to that.

That's fine with me. Thank you for your help so far on reviewing this.

-Jaikiran


>
> 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
>>