[ 
https://issues.apache.org/jira/browse/JCRVLT-339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16885985#comment-16885985
 ] 

Dominik Süß commented on JCRVLT-339:
------------------------------------

[~krystian] as [~tripod] indicated we would in fact rather return a 
vaultpackage but it would contain or return a file.  VaultPackage is already 
defined to be able to return null for the File if the binary is not available.

> 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
>    Affects Versions: 3.2.8
>            Reporter: Krystian Nowak
>            Priority: Major
>         Attachments: JCRVLT-339.patch, JCRVLT-339.patch
>
>
> The logic in 
> [FSRegisteredPackage|https://github.com/apache/jackrabbit-filevault/blob/jackrabbit-filevault-3.2.8/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistry.java]
>  may be improved to handle truncated files proactively before 
> [ZipVaultPackage is instantiated in 
> FSPackageRegistry|https://github.com/apache/jackrabbit-filevault/blob/jackrabbit-filevault-3.2.8/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistry.java#L240]
>  while trying to open it to create _VaultPackage_ in [getPackage 
> method|https://github.com/apache/jackrabbit-filevault/blob/jackrabbit-filevault-3.2.8/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSRegisteredPackage.java#L74].
> This kind of truncation is a mechanism in container based setups to 
> effectively reduce disk usage without functional impact.
> As [getPackage in 
> RegisteredPackage|https://github.com/apache/jackrabbit-filevault/blob/jackrabbit-filevault-3.2.8/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/RegisteredPackage.java#L51]
>  is declared as not returning null (_@Nonnull_ annotation), but throwing 
> _IOException_ - this exceptional situation of truncation in filed-based case 
> might be signalled this way and handled properly by callers aware of the 
> truncation mechanism.
> Currently the exception (in the case of file truncation) is thrown multiple 
> levels lower when accessing _java.util.zip.ZipFile_ with verbose and 
> unconditional 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