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.
