RFR: 8303965: java.net.http.HttpClient should reset the stream if response headers contain malformed header fields [v2]

Daniel Fuchs dfuchs at openjdk.org
Mon Mar 13 11:09:15 UTC 2023


On Sat, 11 Mar 2023 11:44:43 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Daniel Fuchs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - add bug id to test
>>  - Merge branch 'master' into MalformedResponse-8303965
>>  - 8303965
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 1220:
> 
>> 1218:      */
>> 1219:     @SuppressWarnings("unchecked")
>> 1220:     <T> Stream<T> getInitialStream() {
> 
> Given that this method returns and also updates the initial stream member field, the naming of this method is a bit odd. But I can't think of a better name, plus this is internal to the `jdk.internal.net.http` package and also has a comment which explains what it does, so I think this name is fine.

Maybe I could change the name to "retrieveInitialStream()"... Would that be better?

> src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 1603:
> 
>> 1601:                 }
>> 1602:             } catch (UncheckedIOException uio) {
>> 1603:                 // reset stream: From RFC 7540, section-8.1.2.6
> 
> Hello Daniel, should we instead refer to RFC-9113 (section 8.1.1) which obsoletes RFC-7540? In the context of this PR, the newer RFC continues to have the same expectations as that in RFC-7540.

Right. Sorry for the oversight.

> src/java.net.http/share/classes/jdk/internal/net/http/common/ValidatingHeadersConsumer.java line 32:
> 
>> 30: 
>> 31: /*
>> 32:  * Checks RFC 7540 rules (relaxed) compliance regarding pseudo-headers.
> 
> Same here, should we use new RFC number?

Done

-------------

PR: https://git.openjdk.org/jdk/pull/12976


More information about the net-dev mailing list