Review Request: JDK-7142921,(fs) Files.probeContentType reports a MIME type of "text/plain" on Ubuntu 11.04

[Dan Xu]

Hi Alan,

Thanks for your comments. Please see my answers inline.


On 11/26/2012 05:46 AM, Alan Bateman wrote:
> On 26/11/2012 06:40, Dan Xu wrote:
>> Hi,
>>
>> Please help review the code changes for CR 7142921 and CR 7144997. 
>> The webrev is uploaded to, 
>> http://cr.openjdk.java.net/~dxu/7142921/webrev.00/.
>>
>> In the fix, I added two new file type detectors, one is 
>> MimeTypesFileTypeDetector, and the other is 
>> MagicFileTypeDetector.MimeTypesFileTypeDetector is using a file name 
>> extension to look up its MIME type by checking mime.types files, 
>> which can be used in Linux, Solaris, and Mac.MagicFileTypeDetector is 
>> using libmagic to detect a file type in Linux. The library, libmagic, 
>> is provided in the same package of file command, which is currently 
>> only available in Linux platform.
>>
>> Thanks,
>>
>> -Dan
> Thanks for taking this on. I don't have time to do a detailed review 
> of the two new FileTypeDetector implementations but just some initial 
> comments on the the other changes:
>
> 1. I don't think AbstractFileTypeDetector should be changed to check 
> if the file exists and is a directory, maybe you intended this check 
> to be in the libmagic based FileTypeDetector?
This is a shortcut for known MIME types for non-exist files and 
directories for all detectors. GnomeFileTypeDetector and 
MagicFileTypeDetector can detect correctly for those special cases. But 
MimeTypesFileTypeDetector may return wrong results. I can move the 
special check into MimeTypesFileTypeDetector if you think it is a better 
way.
>
> 2. If we are using /etc/mime.types then we might also have to look at 
> ${user.home}/mime.types as I think that can be used to override the 
> mapping in /etc.
/etc/mime.types says "Users can add their own types if they wish by 
creating a ".mime.types"  file in their home directory.  Definitions 
included there will take precedence over those listed here." So I try to 
load ${user.home}/.mime.types before /etc/mime.types. And all files 
loaded earlier take precedence over files loaded later.

>
> 3. It would be good to add a comment to UnixFileSystemProvider.chain. 
> Also if the line length is a problem when you can probably drop 
> protected as the usage is package-private anyway.
I am going to add the comment and remove "protected".
>
> Otherwise the make file updates looks fine. I will get back to you 
> soon on the magic and mimetypes detectors, just can't spend time on it 
> today. The other big thing with this proposal is to agree the priority 
> of the detectors, we also need to consider the diagnosability on each 
> platform (needs to be easy for the user to understand how the file 
> type is determined). Finally, we should double check that 
> /private/etc/apache2/mime.types is the right place to look on Mac.
>
> -Alan
>
>
Thanks,

-Dan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20121126/f8c9ce30/attachment-0001.html