RFR 8241389: URLConnection::getHeaderFields returns result inconsistent with getHeaderField/Key for FileURLConnection, FtpURLConnection

[Michael McMahon]

Thanks for the comments Daniel.

Webrev updated at: http://cr.openjdk.java.net/~michaelm/8241389/webrev.2/

- Michael

On 22/05/2020 15:28, Daniel Fuchs wrote:
> Hi Michael,
>
> Two comments:
>
> URLConnection.java:
>
> 1. I don't think getHeaderFields() should return null,
>    but an empty map instead.
>
>  122                 return null;
>
>    should probably return super.getHeaderFields(); instead.
>
> 2. Other methods in this class seem to assume that
>    `properties` can be null. I haven't tried to figure out
>    whether that's a valid assumption or not but I would
>    advise keeping the pattern for consistency:
>
>
> something like:
>
>  116     @Override
>  117     public Map<String, List<String>> getHeaderFields() {
>              var headerFields = this.headerFields;
>  118         if (headerFields == null) {
>  119             try {
>  120                 getInputStream();
>                      MessageHeader props = this.properties;
>                      if (props != null) {
>                          headerFields = this.headerFields = 
> props.getHeaders();
>                      }
>  121             } catch (IOException e) {
>  122                 // OK
>  123             }
>  125         }
>  126         if (headerFields == null) {
>                  return super.getHeaderFields();
>              } else {
>                  return headerFields;
>              }
>  127     }
>
>
> And maybe add a test case to check that getHeaderFields() returns
> an empty map if getInputStream() throws...
>
> best regards,
>
> -- daniel
>
>
>
>
> On 22/05/2020 14:37, Michael McMahon wrote:
>> Hi,
>>
>> Could I get the following fix reviewed please? It is related
>> to the issue reviewed earlier, but requires a code change
>> instead of a spec update.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8241389
>>
>> Webrev: http://cr.openjdk.java.net/~michaelm/8241389/webrev.1/index.html
>>
>> Thanks,
>>
>> Michael.
>>
>