On Fri, 11 Feb 2022 11:15:56 GMT, Mahendra Chhipa <d...@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