RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v2]

[Brian Burkhalter]

On Sat, 28 May 2022 14:39:16 GMT, Jaikiran Pai <jpai at 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