RFR: JDK-8061729 : Update java/net tests to eliminate dependency on sun.net.www.MessageHeader and some other internal APIs [v5]

[Daniel Fuchs]

On Fri, 11 Feb 2022 11:15:56 GMT, Mahendra Chhipa <duke at openjdk.java.net> wrote:

>> There are some regression tests depending on sun.net.www.MessageHeader, the internal API dependency should be removed. Some of other internal API dependancies are removed in following issues :
>> JDK-8273142
>> JDK-8268464
>> JDK-8268133
>
> Mahendra Chhipa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   1. Handled continuation line
>   2. handled \r, \n, or \r\n
>   3. Added the test for HttpHeaderParser class

test/lib/jdk/test/lib/net/HttpHeaderParser.java line 100:

> 98:     public List<String> getHeaderValue(String key) {
> 99:         if(headerMap.containsKey(key.toLowerCase())) {
> 100:             return headerMap.get(key.toLowerCase());

I'd suggest to use `key.toLowerCase(Locale.ROOT)` - even though it shouldn't really matter if it's guaranteed that keys have been syntactically checked for illegal characters.

test/lib/jdk/test/lib/net/HttpHeaderParser.java line 120:

> 118:      * ( when true is returned ), which may not be fully consumed.
> 119:      *
> 120:      * @param input the ( partial ) header data

It doesn't look like this is a accurate. There's no ByteBuffers anywhere...

test/lib/jdk/test/lib/net/HttpHeaderParser.java line 148:

> 146:             case FINISHED -> false;
> 147:             case STATUS_LINE_FOUND_LF, STATUS_LINE_END_LF, HEADER_FOUND_LF -> true;
> 148:             default -> buffer.available() >= 0;

It's a bit chancy to use InputStream.available() - it would be better to have some `eof` variable that's set to true when EOF is reached, and return `!eof` here

test/lib/jdk/test/lib/net/HttpHeaderParser.java line 162:

> 160:      * value from [0, 255] representing the character code.
> 161:      *
> 162:      * @param input a {@code ByteBuffer} containing a partial input

Input is not a ByteBuffer and it doesn't contain a partial input.

test/lib/jdk/test/lib/net/HttpHeaderParser.java line 171:

> 169:     private char get(InputStream input) throws IOException {
> 170:         return (char)(input.read() & 0xFF);
> 171:     }

That doesn't look right as it will prevent you to read EOF. Maybe it should be:

    boolean eof;
    ...
    private int get(InputStream input) throws IOException {
         int c = input.read();
         if (c < 0) eof = true;
         return c;
    }


And then you would have to check for EOF everywhere where `get(input)` is called too.

test/lib/jdk/test/lib/net/HttpHeaderParser.java line 175:

> 173:     private void readResumeStatusLine(InputStream input) throws IOException {
> 174:         char c = 0;
> 175:         while (input.available() > 0 && (c = get(input)) != CR) {

It's not a too good idea to depend on input.available() - the spec leaves too much room for odd behaviors.
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/io/InputStream.html#available()
An implementation may decide to return 0 always - as it is the amount of bytes that is guaranteed to be readable without blocking (and an implementation may not know how many bytes can be read without blocking).

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

PR: https://git.openjdk.java.net/jdk/pull/5937