On Sat, 28 May 2022 14:39:16 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Brian Burkhalter has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8287237: Refactor and clean up > > src/java.base/share/classes/sun/net/www/MimeTable.java line 188: > >> 186: int hashIndex = fname.lastIndexOf(HASH_MARK); >> 187: if (hashIndex > 0) { >> 188: String ext = getFileExtension(fname.substring(0, hashIndex >> - 1)); > > Hello Brian, I think there's a bug here. Not introduced by this PR but even > in the current master. I think that `fname.substring(0, hashIndex -1)` call > should actually be `fname.substring(0, hashIndex)`. For example, in its > current form, if `fname` is `a.png#foo` then `fname.substring(...)` here will > return `a.pn` instead of `a.png`. It looks like we don't have a test for that > case. I agree. I saw that myself. As nothing seems broken I did not want to touch it but I will investigate. > src/java.base/share/classes/sun/net/www/MimeTable.java line 196: > >> 194: >> 195: String ext = ""; >> 196: if (i != -1 && fname.charAt(i) == '.') > > Nit - the existing method currently uses a `{` even for single line > conditionals. Should we do the same for the new `if` blocks introduced in > this PR and enclose them within `{` `}`? Fixed in eab33e8000ea730c57c918dbf291af23bacbd059. ------------- PR: https://git.openjdk.java.net/jdk/pull/8909