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

Dominik Süß edited comment on JCRVLT-339 at 7/16/19 9:40 AM:
-------------------------------------------------------------

[~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. 
getArchive is not fully descriptive but we can basically treat it similar to 
when it was already closed (also returning null), throw a corresponding 
exception when extract is being called.
If I'm not mistaken we have the full MetaInf & Properties in the registry (will 
have to validate how much information depends on extraction from binary)


was (Author: dsuess):
[~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