chadlwilson commented on code in PR #154:
URL: https://github.com/apache/felix-dev/pull/154#discussion_r882335882


##########
framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java:
##########
@@ -48,16 +49,27 @@ public class DirectoryContent implements Content
     private final File m_rootDir;
     private final File m_dir;
     private Map m_nativeLibMap;
+    private final String m_canonicalRoot;
 
     public DirectoryContent(Logger logger, Map configMap,
-        WeakZipFileFactory zipFactory, Object revisionLock, File rootDir, File 
dir)
+                            WeakZipFileFactory zipFactory, Object 
revisionLock, File rootDir, File dir)
     {
         m_logger = logger;
         m_configMap = configMap;
         m_zipFactory = zipFactory;
         m_revisionLock = revisionLock;
         m_rootDir = rootDir;
         m_dir = dir;
+        String canonicalPath = null;
+        try {
+            canonicalPath = 
BundleCache.getSecureAction().getCanonicalPath(m_dir);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+        if (!canonicalPath.endsWith(File.separator)) {
+            canonicalPath = canonicalPath + "/";

Review Comment:
   Hi there - shouldn't this append `File.separator`, not `/`?
   
   I believe this change is causing things to break with `7.0.4` on Windows 
because when it appends a `/` it no longer passes the later test, where 
`File.separator` is added in the normalization.
   
   ```
   DefaultGoPluginActivatorIntegrationTest > 
shouldRegisterAClassImplementingGoPluginAsAnOSGiService() STANDARD_ERROR
       java.io.IOException: File outside the root: 
C:\tmp\gocd-tests\2197rtj\junit14902581698618482889\DefaultGoPluginActivatorIntegrationTest.bundleDirWhichHasProperActivator\META-INF\MANIFEST.MF
        at 
org.apache.felix.framework.cache.DirectoryContent.getFile(DirectoryContent.java:194)
        at 
org.apache.felix.framework.cache.DirectoryContent.getEntryAsBytes(DirectoryContent.java:143)
        at 
org.apache.felix.framework.util.MultiReleaseContent.wrap(MultiReleaseContent.java:58)
        at 
org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:440)
        at 
org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:380)
        at 
org.apache.felix.framework.BundleRevisionImpl.getContentPath(BundleRevisionImpl.java:362)
        at 
org.apache.felix.framework.BundleWiringImpl$BundleClassLoader.findClass(BundleWiringImpl.java:2038)
        at 
org.apache.felix.framework.BundleWiringImpl.findClassOrResourceByDelegation(BundleWiringImpl.java:1556)
        at 
org.apache.felix.framework.BundleWiringImpl.access$300(BundleWiringImpl.java:79)
        at 
org.apache.felix.framework.BundleWiringImpl$BundleClassLoader.loadClass(BundleWiringImpl.java:1976)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
        at 
org.apache.felix.framework.BundleWiringImpl.getClassByDelegation(BundleWiringImpl.java:1358)
        at 
org.apache.felix.framework.Felix.createBundleActivator(Felix.java:4774)
        at org.apache.felix.framework.Felix.activateBundle(Felix.java:2410)
        at org.apache.felix.framework.Felix.startBundle(Felix.java:2335)
        at org.apache.felix.framework.BundleImpl.start(BundleImpl.java:1006)
        at org.apache.felix.framework.BundleImpl.start(BundleImpl.java:992)
        at 
com.thoughtworks.go.plugin.infra.FelixGoPluginOSGiFramework.loadPlugin(FelixGoPluginOSGiFramework.java:99)
        at 
com.thoughtworks.go.plugin.activation.DefaultGoPluginActivatorIntegrationTest.installBundleFoundInDirectory(DefaultGoPluginActivatorIntegrationTest.java:370)
        at 
com.thoughtworks.go.plugin.activation.DefaultGoPluginActivatorIntegrationTest.installBundleWithClasses(DefaultGoPluginActivatorIntegrationTest.java:358)
        at 
com.thoughtworks.go.plugin.activation.DefaultGoPluginActivatorIntegrationTest.assertThatPluginWithThisExtensionClassLoadsSuccessfully(DefaultGoPluginActivatorIntegrationTest.java:319)
        at 
com.thoughtworks.go.plugin.activation.DefaultGoPluginActivatorIntegrationTest.shouldRegisterAClassImplementingGoPluginAsAnOSGiService(DefaultGoPluginActivatorIntegrationTest.java:83)
   // snipped
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to