Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 335190

Summary: ParBridge.getApplicationVersion breaks if there is a trailing space on the version
Product: [RT] Virgo Reporter: Daniel Mikusa <dmikusa>
Component: runtimeAssignee: Hristo Iliev <hsiliev>
Status: CLOSED FIXED QA Contact:
Severity: trivial    
Priority: P3 CC: glyn.normington, hsiliev
Version: 2.1.0.RELEASE   
Target Milestone: 3.0.0.M02   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
modified PAR file to replicate exception. none

Description Daniel Mikusa CLA 2011-01-24 09:21:48 EST
Created attachment 187422 [details]
modified PAR file to replicate exception.

If the version number specified in a PAR file's MANIFEST has a trailing space on the "Application-Version" then an exception is thrown and the incorrect version is used.

Example:

In the "org.eclipse.virgo.apps.repository-2.1.0.RELEASE.par" which is distributed with Virgo, I altered the MANIFEST file so that the "Application-Version" reads "2.1.0.RELEASE " (rather than "2.1.0.RELEASE").

When the bundle deploys, you see the following in the log file which indicates that version "0.0.0" was detected (incorrect).

[2011-01-24 09:07:20.953] start-signalling-1           <DE0005I> Started par 'org.eclipse.virgo.apps.repository-2.1.0.RELEASE' version '0.0.0'. 

Also, in the log file you see the following exception which indicates that the extra space has caused a problem.

[2011-01-24 09:07:17.425] deployer-recovery            o.e.v.k.i.artifact.internal.StandardArtifactIdentityDeterminer    Error occurred while determining the type of an Artifact '/home/daniel/Development/servers/virgo-web-server-2.1.0.RELEASE/pickup/org.eclipse.virgo.apps.repository-2.1.0.RELEASE.par' with the bridge '$Proxy72'. org.eclipse.virgo.repository.ArtifactGenerationException: Version '2.1.0.RELEASE ' is ill-formed
        at org.eclipse.virgo.kernel.artifact.par.ParBridge.getApplicationVersion(ParBridge.java:115)
        at org.eclipse.virgo.kernel.artifact.par.ParBridge.createDescriptorFromManifest(ParBridge.java:84)
        at org.eclipse.virgo.kernel.artifact.par.ParBridge.generateArtifactDescriptor(ParBridge.java:70)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
        at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:307)
        at org.springframework.osgi.service.importer.support.internal.aop.ServiceInvoker.doInvoke(ServiceInvoker.java:58)
        at org.springframework.osgi.service.importer.support.internal.aop.ServiceInvoker.invoke(ServiceInvoker.java:62)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
        at org.springframework.aop.support.DelegatingIntroductionInterceptor.doProceed(DelegatingIntroductionInterceptor.java:131)
        at org.springframework.aop.support.DelegatingIntroductionInterceptor.invoke(DelegatingIntroductionInterceptor.java:119)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
        at org.springframework.osgi.service.util.internal.aop.ServiceTCCLInterceptor.invokeUnprivileged(ServiceTCCLInterceptor.java:56)
        at org.springframework.osgi.service.util.internal.aop.ServiceTCCLInterceptor.invoke(ServiceTCCLInterceptor.java:39)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
        at org.springframework.osgi.service.importer.support.LocalBundleContextAdvice.invoke(LocalBundleContextAdvice.java:59)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
        at org.springframework.aop.support.DelegatingIntroductionInterceptor.doProceed(DelegatingIntroductionInterceptor.java:131)
        at org.springframework.aop.support.DelegatingIntroductionInterceptor.invoke(DelegatingIntroductionInterceptor.java:119)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
        at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:202)
        at $Proxy72.generateArtifactDescriptor(Unknown Source)
        at org.eclipse.virgo.kernel.install.artifact.internal.StandardArtifactIdentityDeterminer.determineIdentity(StandardArtifactIdentityDeterminer.java:97)
        at org.eclipse.virgo.kernel.install.artifact.internal.DelegatingServiceRegistryBackedArtifactIdentityDeterminer.determineIdentity(DelegatingServiceRegistryBackedArtifactIdentityDeterminer.java:60)
        at org.eclipse.virgo.kernel.install.artifact.internal.StandardInstallArtifactTreeInclosure.determineIdentity(StandardInstallArtifactTreeInclosure.java:135)
        at org.eclipse.virgo.kernel.install.artifact.internal.StandardInstallArtifactTreeInclosure.recoverInstallTree(StandardInstallArtifactTreeInclosure.java:180)
        at org.eclipse.virgo.kernel.deployer.core.internal.PipelinedApplicationDeployer.recoverDeployment(PipelinedApplicationDeployer.java:248)
        at org.eclipse.virgo.kernel.deployer.core.internal.recovery.RecoveryAgent$1.run(RecoveryAgent.java:71)
        at java.lang.Thread.run(Thread.java:662)
Caused by: java.lang.IllegalArgumentException: invalid qualifier: RELEASE 
        at org.osgi.framework.Version.validate(Version.java:188)
        at org.osgi.framework.Version.<init>(Version.java:154)
        at org.eclipse.virgo.kernel.artifact.par.ParBridge.getApplicationVersion(ParBridge.java:113)
        ... 30 common frames omitted


While it is easy enough to remove the extra space and correct the problem, it would be much friendlier if Virgo could ignore that space.  If that is not possible then a more meaningful error message would nice.

Example:

Version '2.1.0.RELEASE ' is ill-formed, trailing white space is not allowed.

Thanks

ATTACHED: modified PAR file to replicate exception.
Comment 1 Glyn Normington CLA 2011-01-25 05:05:55 EST
We should continue to reject invalid qualifiers as being permissive here opens the door to unexpected parsing results. However, if we can improve the diagnostics, we should do.
Comment 2 Hristo Iliev CLA 2011-01-27 06:28:22 EST
The current message cannot be easily improved.

The PAR version is set to "empty" or 0.0.0 if the one in the manifest cannot be parsed. Therefore the code sees this as a valid version and simply outputs it.

What can be done:
1) provide additional console message if the version cannot be parsed
2) refuse to install the PAR since it has invalid metadata

What do you think?
Comment 3 Glyn Normington CLA 2011-02-11 04:26:20 EST
I think we should do both 1 and 2.
Comment 4 Daniel Mikusa CLA 2011-02-11 08:49:41 EST
I think both 1 and 2 would be very helpful.  

If the PAR file does not deploy then users will be required to investigate further.  Upon investigation users should see the console message which will indicate the problem with parsing the version.
Comment 5 Hristo Iliev CLA 2011-02-28 08:27:07 EST
1) additional console message is provided if the version cannot be parsed
2) PAR is not installed if it has invalid version

Added with commits 5f9a0b8fac280f9acf359d76fd50bbf5b4e148b5 and 335a868a608cc366130b8adb608f73fb96b135ec