[ 
https://issues.apache.org/jira/browse/JCRVLT-339?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Krystian Nowak updated JCRVLT-339:
----------------------------------
    Description: 
The logic in FSRegisteredPackage may be improved to handle truncated files 
proactively before instantiating ZipVaultPackage while trying to open them to 
create VaultPackage in getPackage method.

This kind of truncation is a mechanism in container based setups to effectively 
reduce disk usage without functional impact.
As getPackage is declared as not returning null, but throwing IOException - 
this exceptional situation might be signalled this way and handled properly by 
callers aware of the truncation mechanism.

Currently the exception is thrown multiple levels lower when accessing 
java.util.zip.ZipFile with verbose logging on multiple levels as the one given 
below:

{noformat}
12.07.2019 15:06:48.730 *ERROR* [MyThread1] 
org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage Error while loading 
package /foo/bar/baz.zip.
12.07.2019 15:06:48.732 *ERROR* [MyThread1] 
org.apache.jackrabbit.vault.packaging.registry.impl.FSPackageRegistry Cloud not 
open file /foo/bar/baz.zip as ZipVaultPackage.
 java.util.zip.ZipException: zip file is empty
        at java.util.zip.ZipFile$Source.zerror(ZipFile.java:1535)
        at java.util.zip.ZipFile$Source.findEND(ZipFile.java:1349)
        at java.util.zip.ZipFile$Source.initCEN(ZipFile.java:1443)
        at java.util.zip.ZipFile$Source.<init>(ZipFile.java:1274)
        at java.util.zip.ZipFile$Source.get(ZipFile.java:1237)
        at java.util.zip.ZipFile$CleanableResource.<init>(ZipFile.java:727)
        at java.util.zip.ZipFile$CleanableResource.get(ZipFile.java:844)
        at java.util.zip.ZipFile.<init>(ZipFile.java:247)
        at java.util.zip.ZipFile.<init>(ZipFile.java:177)
        at java.util.jar.JarFile.<init>(JarFile.java:346)
        at java.util.jar.JarFile.<init>(JarFile.java:317)
        at java.util.jar.JarFile.<init>(JarFile.java:283)
        at 
org.apache.jackrabbit.vault.fs.io.ZipArchive.open(ZipArchive.java:103) 
[org.apache.jackrabbit.vault:3.2.8]
        at 
org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage.<init>(ZipVaultPackage.java:69)
 [org.apache.jackrabbit.vault:3.2.8]
        at 
org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage.<init>(ZipVaultPackage.java:61)
 [org.apache.jackrabbit.vault:3.2.8]
        at 
org.apache.jackrabbit.vault.packaging.registry.impl.FSPackageRegistry.open(FSPackageRegistry.java:240)
 [org.apache.jackrabbit.vault:3.2.8]
        at 
org.apache.jackrabbit.vault.packaging.registry.impl.FSRegisteredPackage.getPackage(FSRegisteredPackage.java:76)
 [org.apache.jackrabbit.vault:3.2.8]
(...)
{noformat}

Instead a check for underlying file existence and size might be useful here and 
the responsibility to log (or handle) the exception might be put on the caller 
of getPackage method of FSRegisteredPackage (as per method's throws 
declaration) as the one below:

{noformat}
diff --git 
a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
 
b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
index 97416a4..2d18719 100644
--- 
a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
+++ 
b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
@@ -16,6 +16,7 @@
  */
 package org.apache.jackrabbit.vault.packaging.registry.impl;
 
+import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Calendar;
@@ -73,7 +74,12 @@ public class FSRegisteredPackage implements 
RegisteredPackage {
     @Override
     public VaultPackage getPackage() throws IOException {
         if (this.vltPkg == null) {
-            this.vltPkg = registry.open(filepath.toFile());
+            File file = filepath.toFile();
+            if(file.exists() && file.length() > 0) {
+                this.vltPkg = registry.open(file);
+            } else {
+                throw new IOException("underlying file " + filepath.toString() 
+ " for package " + getId().toString() + " is inaccessible - it might be 
truncated");
+            }
         }
         return this.vltPkg;
     }
{noformat}

(see also [^JCRVLT-339.patch] attached and PR: 
[https://github.com/apache/jackrabbit-filevault/pull/50])

Similar approach connected with underlying VLT package file truncation in 
container based setups can be find in SLING-8105 and its corresponding change: 
[https://github.com/apache/sling-org-apache-sling-feature-extension-content/commit/9eecc6d2137c8cafa70edcfd3faa839ed4412f4e]

  was:
The logic in FSRegisteredPackage may be improved to handle truncated files 
proactively before instantiating ZipVaultPackage while trying to open them to 
create VaultPackage in getPackage method.

This kind of truncation is a mechanism in container based setups to effectively 
reduce disk usage without functional impact.
As getPackage is declared as not returning null, but throwing IOException - 
this exceptional situation might be signalled this way and handled properly by 
callers aware of the truncation mechanism.

Currently the exception is thrown multiple levels lower when accessing 
java.util.zip.ZipFile with verbose logging on multiple levels as the one given 
below:

{noformat}
12.07.2019 15:06:48.730 *ERROR* [MyThread1] 
org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage Error while loading 
package /foo/bar/baz.zip.
12.07.2019 15:06:48.732 *ERROR* [MyThread1] 
org.apache.jackrabbit.vault.packaging.registry.impl.FSPackageRegistry Cloud not 
open file /foo/bar/baz.zip as ZipVaultPackage.
 java.util.zip.ZipException: zip file is empty
        at java.util.zip.ZipFile$Source.zerror(ZipFile.java:1535)
        at java.util.zip.ZipFile$Source.findEND(ZipFile.java:1349)
        at java.util.zip.ZipFile$Source.initCEN(ZipFile.java:1443)
        at java.util.zip.ZipFile$Source.<init>(ZipFile.java:1274)
        at java.util.zip.ZipFile$Source.get(ZipFile.java:1237)
        at java.util.zip.ZipFile$CleanableResource.<init>(ZipFile.java:727)
        at java.util.zip.ZipFile$CleanableResource.get(ZipFile.java:844)
        at java.util.zip.ZipFile.<init>(ZipFile.java:247)
        at java.util.zip.ZipFile.<init>(ZipFile.java:177)
        at java.util.jar.JarFile.<init>(JarFile.java:346)
        at java.util.jar.JarFile.<init>(JarFile.java:317)
        at java.util.jar.JarFile.<init>(JarFile.java:283)
        at 
org.apache.jackrabbit.vault.fs.io.ZipArchive.open(ZipArchive.java:103) 
[org.apache.jackrabbit.vault:3.2.8]
        at 
org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage.<init>(ZipVaultPackage.java:69)
 [org.apache.jackrabbit.vault:3.2.8]
        at 
org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage.<init>(ZipVaultPackage.java:61)
 [org.apache.jackrabbit.vault:3.2.8]
        at 
org.apache.jackrabbit.vault.packaging.registry.impl.FSPackageRegistry.open(FSPackageRegistry.java:240)
 [org.apache.jackrabbit.vault:3.2.8]
        at 
org.apache.jackrabbit.vault.packaging.registry.impl.FSRegisteredPackage.getPackage(FSRegisteredPackage.java:76)
 [org.apache.jackrabbit.vault:3.2.8]
(...)
{noformat}

Instead a check for underlying file existence and size might be useful here and 
the responsibility to log (or handle) the exception might be put on the caller 
of getPackage method of FSRegisteredPackage (as per method's throws 
declaration) as the one below:

{noformat}
diff --git 
a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
 
b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
index 97416a4..2d18719 100644
--- 
a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
+++ 
b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
@@ -16,6 +16,7 @@
  */
 package org.apache.jackrabbit.vault.packaging.registry.impl;
 
+import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Calendar;
@@ -73,7 +74,12 @@ public class FSRegisteredPackage implements 
RegisteredPackage {
     @Override
     public VaultPackage getPackage() throws IOException {
         if (this.vltPkg == null) {
-            this.vltPkg = registry.open(filepath.toFile());
+            File file = filepath.toFile();
+            if(file.exists() && file.length() > 0) {
+                this.vltPkg = registry.open(file);
+            } else {
+                throw new IOException("underlying file " + filepath.toString() 
+ " for package " + getId().toString() + " is inaccessible - it might be 
truncated");
+            }
         }
         return this.vltPkg;
     }
{noformat}

(see also [^JCRVLT-339.patch] attached)

Similar approach connected with underlying VLT package file truncation in 
container based setups can be find in SLING-8105 and its corresponding change: 
[https://github.com/apache/sling-org-apache-sling-feature-extension-content/commit/9eecc6d2137c8cafa70edcfd3faa839ed4412f4e]


> Make FSRegisteredPackage able to handle truncated files proactively
> -------------------------------------------------------------------
>
>                 Key: JCRVLT-339
>                 URL: https://issues.apache.org/jira/browse/JCRVLT-339
>             Project: Jackrabbit FileVault
>          Issue Type: Improvement
>          Components: Packaging
>            Reporter: Krystian Nowak
>            Priority: Major
>         Attachments: JCRVLT-339.patch
>
>
> The logic in FSRegisteredPackage may be improved to handle truncated files 
> proactively before instantiating ZipVaultPackage while trying to open them to 
> create VaultPackage in getPackage method.
> This kind of truncation is a mechanism in container based setups to 
> effectively reduce disk usage without functional impact.
> As getPackage is declared as not returning null, but throwing IOException - 
> this exceptional situation might be signalled this way and handled properly 
> by callers aware of the truncation mechanism.
> Currently the exception is thrown multiple levels lower when accessing 
> java.util.zip.ZipFile with verbose logging on multiple levels as the one 
> given below:
> {noformat}
> 12.07.2019 15:06:48.730 *ERROR* [MyThread1] 
> org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage Error while 
> loading package /foo/bar/baz.zip.
> 12.07.2019 15:06:48.732 *ERROR* [MyThread1] 
> org.apache.jackrabbit.vault.packaging.registry.impl.FSPackageRegistry Cloud 
> not open file /foo/bar/baz.zip as ZipVaultPackage.
>  java.util.zip.ZipException: zip file is empty
>       at java.util.zip.ZipFile$Source.zerror(ZipFile.java:1535)
>       at java.util.zip.ZipFile$Source.findEND(ZipFile.java:1349)
>       at java.util.zip.ZipFile$Source.initCEN(ZipFile.java:1443)
>       at java.util.zip.ZipFile$Source.<init>(ZipFile.java:1274)
>       at java.util.zip.ZipFile$Source.get(ZipFile.java:1237)
>       at java.util.zip.ZipFile$CleanableResource.<init>(ZipFile.java:727)
>       at java.util.zip.ZipFile$CleanableResource.get(ZipFile.java:844)
>       at java.util.zip.ZipFile.<init>(ZipFile.java:247)
>       at java.util.zip.ZipFile.<init>(ZipFile.java:177)
>       at java.util.jar.JarFile.<init>(JarFile.java:346)
>       at java.util.jar.JarFile.<init>(JarFile.java:317)
>       at java.util.jar.JarFile.<init>(JarFile.java:283)
>       at 
> org.apache.jackrabbit.vault.fs.io.ZipArchive.open(ZipArchive.java:103) 
> [org.apache.jackrabbit.vault:3.2.8]
>       at 
> org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage.<init>(ZipVaultPackage.java:69)
>  [org.apache.jackrabbit.vault:3.2.8]
>       at 
> org.apache.jackrabbit.vault.packaging.impl.ZipVaultPackage.<init>(ZipVaultPackage.java:61)
>  [org.apache.jackrabbit.vault:3.2.8]
>       at 
> org.apache.jackrabbit.vault.packaging.registry.impl.FSPackageRegistry.open(FSPackageRegistry.java:240)
>  [org.apache.jackrabbit.vault:3.2.8]
>       at 
> org.apache.jackrabbit.vault.packaging.registry.impl.FSRegisteredPackage.getPackage(FSRegisteredPackage.java:76)
>  [org.apache.jackrabbit.vault:3.2.8]
> (...)
> {noformat}
> Instead a check for underlying file existence and size might be useful here 
> and the responsibility to log (or handle) the exception might be put on the 
> caller of getPackage method of FSRegisteredPackage (as per method's throws 
> declaration) as the one below:
> {noformat}
> diff --git 
> a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
>  
> b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
> index 97416a4..2d18719 100644
> --- 
> a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
> +++ 
> b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java
> @@ -16,6 +16,7 @@
>   */
>  package org.apache.jackrabbit.vault.packaging.registry.impl;
>  
> +import java.io.File;
>  import java.io.IOException;
>  import java.nio.file.Path;
>  import java.util.Calendar;
> @@ -73,7 +74,12 @@ public class FSRegisteredPackage implements 
> RegisteredPackage {
>      @Override
>      public VaultPackage getPackage() throws IOException {
>          if (this.vltPkg == null) {
> -            this.vltPkg = registry.open(filepath.toFile());
> +            File file = filepath.toFile();
> +            if(file.exists() && file.length() > 0) {
> +                this.vltPkg = registry.open(file);
> +            } else {
> +                throw new IOException("underlying file " + 
> filepath.toString() + " for package " + getId().toString() + " is 
> inaccessible - it might be truncated");
> +            }
>          }
>          return this.vltPkg;
>      }
> {noformat}
> (see also [^JCRVLT-339.patch] attached and PR: 
> [https://github.com/apache/jackrabbit-filevault/pull/50])
> Similar approach connected with underlying VLT package file truncation in 
> container based setups can be find in SLING-8105 and its corresponding 
> change: 
> [https://github.com/apache/sling-org-apache-sling-feature-extension-content/commit/9eecc6d2137c8cafa70edcfd3faa839ed4412f4e]



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

Reply via email to