You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/06/10 04:03:41 UTC
[GitHub] [commons-vfs] PeterAlfredLee opened a new pull request #92: [VFS-770] Gz createFileSystem fails
PeterAlfredLee opened a new pull request #92:
URL: https://github.com/apache/commons-vfs/pull/92
The `.gz` is parsed as `application/octet-stream` by mime type and could not be successfully handled.
This PR is a fix for this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] PeterAlfredLee commented on a change in pull request #92: [VFS-770] Gz createFileSystem fails
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on a change in pull request #92:
URL: https://github.com/apache/commons-vfs/pull/92#discussion_r438056066
##########
File path: commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/test/DefaultFileSystemManagerTest.java
##########
@@ -63,4 +68,22 @@ public void testResolveFileObjectNullAbsolute() throws FileSystemException {
public void testResolveFileNameNull() throws FileSystemException {
VFS.getManager().resolveName((FileName) null, "../");
}
+
+
+ @Test
+ public void testCreateGzipFileSystem() throws IOException {
+ // setup .gz file for test
+ final File gzFile = new File("src/test/resources/test-data/test.gz");
+ final File newGzFile = File.createTempFile(getClass().getSimpleName(), ".gz");
+ newGzFile.deleteOnExit();
+ FileUtils.copyFile(gzFile, newGzFile);
+ FileSystemManager manager = VFS.getManager();
Review comment:
Sure. Will push again avoiding copying the file.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] garydgregory commented on a change in pull request #92: [VFS-770] Gz createFileSystem fails
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #92:
URL: https://github.com/apache/commons-vfs/pull/92#discussion_r438478951
##########
File path: commons-vfs2/src/main/java/org/apache/commons/vfs2/impl/FileTypeMap.java
##########
@@ -57,7 +58,7 @@ public String getScheme(final FileObject file) throws FileSystemException {
// Check the file's mime type for a match
final FileContent content = file.getContent();
final String mimeType = content.getContentInfo().getContentType();
- if (mimeType != null) {
+ if (mimeType != null && !mimeType.equals(MIME_TYPE_OCTET_STREAM)) {
Review comment:
This does not feel right. One the one hand we tell caller to call `addMimeType(String, String)` but then we ignore that entry for one specific value. That does not make sense. I'd like to hear from others who might be more familiar with this part of the code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] garydgregory commented on pull request #92: [VFS-770] Gz createFileSystem fails
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #92:
URL: https://github.com/apache/commons-vfs/pull/92#issuecomment-788047232
Hi @PeterAlfredLee
May you please rebase on master?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] PeterAlfredLee commented on a change in pull request #92: [VFS-770] Gz createFileSystem fails
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on a change in pull request #92:
URL: https://github.com/apache/commons-vfs/pull/92#discussion_r438501370
##########
File path: commons-vfs2/src/main/java/org/apache/commons/vfs2/impl/FileTypeMap.java
##########
@@ -57,7 +58,7 @@ public String getScheme(final FileObject file) throws FileSystemException {
// Check the file's mime type for a match
final FileContent content = file.getContent();
final String mimeType = content.getContentInfo().getContentType();
- if (mimeType != null) {
+ if (mimeType != null && !mimeType.equals(MIME_TYPE_OCTET_STREAM)) {
Review comment:
Actually `application/octet-stream` is the default value for extensions not in the [mime type table](https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types). I'm just adding one more extension check using `VFS's extensionMap` if we encountered a `application/octet-stream` here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] PeterAlfredLee commented on a change in pull request #92: [VFS-770] Gz createFileSystem fails
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on a change in pull request #92:
URL: https://github.com/apache/commons-vfs/pull/92#discussion_r438499177
##########
File path: commons-vfs2/src/main/java/org/apache/commons/vfs2/impl/FileTypeMap.java
##########
@@ -57,7 +58,7 @@ public String getScheme(final FileObject file) throws FileSystemException {
// Check the file's mime type for a match
final FileContent content = file.getContent();
final String mimeType = content.getContentInfo().getContentType();
- if (mimeType != null) {
+ if (mimeType != null && !mimeType.equals(MIME_TYPE_OCTET_STREAM)) {
Review comment:
Not familiar with `mimeType` eithor, seems it's used in Internet media.
According to the [mimeType table](https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types) the `.gz` should be regarded as `application/gzip`, but we got a `application/octet-stream` here. Actually I diged a little here and found it's caused in `sun.net.www.MimeTable` and will treat extensions `.saveme, .dump, .hqx, .arc, .obj, .lib, .bin, .exe, .gz` as `application/octet-stream`.Not sure if this is a potential problem of `sun.net.www.MimeTable` or not.
This is just a workaround for it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] garydgregory commented on a change in pull request #92: [VFS-770] Gz createFileSystem fails
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #92:
URL: https://github.com/apache/commons-vfs/pull/92#discussion_r438026718
##########
File path: commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/test/DefaultFileSystemManagerTest.java
##########
@@ -63,4 +68,22 @@ public void testResolveFileObjectNullAbsolute() throws FileSystemException {
public void testResolveFileNameNull() throws FileSystemException {
VFS.getManager().resolveName((FileName) null, "../");
}
+
+
+ @Test
+ public void testCreateGzipFileSystem() throws IOException {
+ // setup .gz file for test
+ final File gzFile = new File("src/test/resources/test-data/test.gz");
+ final File newGzFile = File.createTempFile(getClass().getSimpleName(), ".gz");
+ newGzFile.deleteOnExit();
+ FileUtils.copyFile(gzFile, newGzFile);
+ FileSystemManager manager = VFS.getManager();
Review comment:
May you please either add a comment as to why this test needs its own copy of the file, or simplify the test. Can't the test use the file fixture as it stands? Assuming the test releasing resources works and there are no bugs at that level of course.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org