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

Karl Pauls commented on SLING-10790:
------------------------------------

[~cziegeler], I agree that should be the case - however, we had cases where a 
bundle didn't have both. Furthermore, I seem to remember cases where there was 
more than one pom. Ultimately, I'm not sure the classifier would be necessarily 
reflected in the bundle name. 

I guess that is part of why the code is so complicated, it tries to find the 
pom properties, if not, it tries to see if a pom is there that seems to match 
(however, it seems like it has some potential ordering problems in that 
assuming it is possible to have the order of the entries in a way where a 
matching pom comes first that isn't the right one). 

Cleaning it up makes a lot of sense - just wanted to point out what I remember 
from previous issues.

> BundleEntryHandler.extractArtifactId may use wrong GAV
> ------------------------------------------------------
>
>                 Key: SLING-10790
>                 URL: https://issues.apache.org/jira/browse/SLING-10790
>             Project: Sling
>          Issue Type: Bug
>          Components: Content-Package to Feature Model Converter
>            Reporter: Angela Schreiber
>            Priority: Minor
>             Fix For: Content-Package to Feature Model Converter 1.1.12
>
>
> [~kpauls], if my reading of {{BundleEntryHandler.extractArtifactId}} is 
> correct it the method might be ending up using the wrong 
> groupId/artifactId/version.
> the code will loop over jar-entries and stop if the extracted GAV matches the 
> bundle name. however, groupId/artifactId/version are not reset to {{null}} in 
> case they were successfully extracted but didn't end up matching the bundle 
> name i.e. {quote}it was the pom.properties  we were looking for{quote}.
> i can't tell how big of an issue that is (and how likely). but given the fact 
> that there is some extra effort to verify that the parsed pom is actually the 
> right one, it might actually be relevant. the relies on a compliant content 
> package that does contain a matching pom, which may or may not be the case... 
> logging a warning or throwing a ConverterException in case of violation might 
> help spotting troublesome content packages instead of getting some sort of 
> side effect if another pom was spotted.
> a heavily simplified copy of the method:
> {code}
>         String artifactId = null;
>         String version = null;
>         String groupId = null;
>         String classifier = null;
>         for (Enumeration<JarEntry> e = jarFile.entries(); 
> e.hasMoreElements();) {
>             [...]
>             // extract groupId/artifactId/version
>             [...]
>        
>             if (groupId != null && artifactId != null && version != null) {
>                 // bundleName is now the bare name without extension
>                 String synthesized = artifactId + "-" + version;
>                 // it was the pom.properties  we were looking for
>                 if (bundleName.startsWith(synthesized) || 
> bundleName.equals(artifactId)) {
>                     [...]
>                     
>                     // no need to iterate further
>                     break;
>                 }
>             }
>         }
>         
>         if (groupId == null) {
>             [...]
>         }
>         return new ArtifactId(groupId, artifactId, version, classifier, 
> JAR_TYPE);
> {code}
> feel free to resolve as not a problem in case my reading of the code is all 
> wrong.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to