Skip to content

Commit c2da9a5

Browse files
karlpaulskarlpauls
authored
Felix-6535: Cleanup directory revision exception handling (#154)
1 parent 2ced71c commit c2da9a5

2 files changed

Lines changed: 116 additions & 44 deletions

File tree

‎framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java‎

Lines changed: 105 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.io.File;
2828
import java.io.IOException;
2929
import java.io.InputStream;
30+
import java.io.UncheckedIOException;
3031
import java.net.MalformedURLException;
3132
import java.net.URL;
3233
import java.util.Enumeration;
@@ -48,16 +49,27 @@ public class DirectoryContent implements Content
4849
private final File m_rootDir;
4950
private final File m_dir;
5051
private Map m_nativeLibMap;
52+
private final String m_canonicalRoot;
5153

5254
public DirectoryContent(Logger logger, Map configMap,
53-
WeakZipFileFactory zipFactory, Object revisionLock, File rootDir, File dir)
55+
WeakZipFileFactory zipFactory, Object revisionLock, File rootDir, File dir)
5456
{
5557
m_logger = logger;
5658
m_configMap = configMap;
5759
m_zipFactory = zipFactory;
5860
m_revisionLock = revisionLock;
5961
m_rootDir = rootDir;
6062
m_dir = dir;
63+
String canonicalPath = null;
64+
try {
65+
canonicalPath = BundleCache.getSecureAction().getCanonicalPath(m_dir);
66+
} catch (IOException e) {
67+
throw new UncheckedIOException(e);
68+
}
69+
if (!canonicalPath.endsWith(File.separator)) {
70+
canonicalPath = canonicalPath + "/";
71+
}
72+
m_canonicalRoot = canonicalPath;
6173
}
6274

6375
public File getFile()
@@ -77,9 +89,17 @@ public boolean hasEntry(String name) throws IllegalStateException
7789
// Return true if the file associated with the entry exists,
7890
// unless the entry name ends with "/", in which case only
7991
// return true if the file is really a directory.
80-
File file = new File(m_dir, name);
92+
File file = null;
93+
try
94+
{
95+
file = getFile(name);
96+
}
97+
catch (IOException e)
98+
{
99+
return false;
100+
}
81101
return BundleCache.getSecureAction().fileExists(file)
82-
&& (name.endsWith("/")
102+
&& (name.endsWith("/")
83103
? BundleCache.getSecureAction().isFileDirectory(file) : true);
84104
}
85105

@@ -91,7 +111,15 @@ public boolean isDirectory(String name)
91111
// Return true if the file associated with the entry exists,
92112
// unless the entry name ends with "/", in which case only
93113
// return true if the file is really a directory.
94-
File file = new File(m_dir, name);
114+
File file = null;
115+
try
116+
{
117+
file = getFile(name);
118+
}
119+
catch (IOException e)
120+
{
121+
return false;
122+
}
95123
return BundleCache.getSecureAction().isFileDirectory(file);
96124
}
97125

@@ -110,36 +138,36 @@ public byte[] getEntryAsBytes(String name) throws IllegalStateException
110138

111139
// Get the embedded resource.
112140

113-
File file = new File(m_dir, name);
114141
try
115142
{
143+
File file = getFile(name);
116144

117145
return BundleCache.getSecureAction().fileExists(file) ? BundleCache.read(BundleCache.getSecureAction().getInputStream(file), file.length()) : null;
118146
}
119147
catch (Exception ex)
120148
{
121149
m_logger.log(
122-
Logger.LOG_ERROR,
123-
"DirectoryContent: Unable to read bytes for file " + name + " from file " + file.getAbsolutePath(), ex);
150+
Logger.LOG_ERROR,
151+
"DirectoryContent: Unable to read bytes for file " + name + " from file " + new File(m_dir, name).getAbsolutePath(), ex);
124152
return null;
125153
}
126154
}
127155

128156
public InputStream getEntryAsStream(String name)
129-
throws IllegalStateException, IOException
157+
throws IllegalStateException, IOException
130158
{
131159
name = getName(name);
132160

133-
File file = new File(m_dir, name);
134161
try
135162
{
163+
File file = getFile(name);
136164
return BundleCache.getSecureAction().fileExists(file) ? BundleCache.getSecureAction().getInputStream(file) : null;
137165
}
138166
catch (Exception ex)
139167
{
140168
m_logger.log(
141-
Logger.LOG_ERROR,
142-
"DirectoryContent: Unable to create inputstream for file " + name + " from file " + file.getAbsolutePath(), ex);
169+
Logger.LOG_ERROR,
170+
"DirectoryContent: Unable to create inputstream for file " + name + " from file " + new File(m_dir, name).getAbsolutePath(), ex);
143171
return null;
144172
}
145173
}
@@ -152,6 +180,22 @@ private String getName(String name)
152180
return name;
153181
}
154182

183+
private File getFile(String name) throws IOException
184+
{
185+
File result = new File(m_dir, name);
186+
187+
String canonicalPath = BundleCache.getSecureAction().getCanonicalPath(result);
188+
if (BundleCache.getSecureAction().isFileDirectory(result) && !canonicalPath.endsWith(File.separator))
189+
{
190+
canonicalPath += File.separator;
191+
}
192+
if (!canonicalPath.startsWith(m_canonicalRoot))
193+
{
194+
throw new IOException("File outside the root: " + canonicalPath);
195+
}
196+
return result;
197+
}
198+
155199
public URL getEntryAsURL(String name)
156200
{
157201
name = getName(name);
@@ -160,9 +204,9 @@ public URL getEntryAsURL(String name)
160204
{
161205
try
162206
{
163-
return BundleCache.getSecureAction().toURI(new File(m_dir, name)).toURL();
207+
return BundleCache.getSecureAction().toURI(getFile(name)).toURL();
164208
}
165-
catch (MalformedURLException e)
209+
catch (IOException e)
166210
{
167211
return null;
168212
}
@@ -178,7 +222,12 @@ public long getContentTime(String name)
178222
{
179223
name = getName(name);
180224

181-
File file = new File(m_dir, name);
225+
File file = null;
226+
try {
227+
file = getFile(name);
228+
} catch (IOException e) {
229+
return 0L;
230+
}
182231

183232
return BundleCache.getSecureAction().getLastModified(file);
184233
}
@@ -190,51 +239,56 @@ public Content getEntryAsContent(String entryName)
190239
if (entryName.equals(FelixConstants.CLASS_PATH_DOT))
191240
{
192241
return new DirectoryContent(
193-
m_logger, m_configMap, m_zipFactory, m_revisionLock, m_rootDir, m_dir);
242+
m_logger, m_configMap, m_zipFactory, m_revisionLock, m_rootDir, m_dir);
194243
}
195244

196245
// Remove any leading slash, since all bundle class path
197246
// entries are relative to the root of the bundle.
198247
entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName;
199248

200249
if (entryName.trim().startsWith(".." + File.separatorChar) ||
201-
entryName.contains(File.separator + ".." + File.separatorChar) ||
202-
entryName.trim().endsWith(File.separator + "..") ||
203-
entryName.trim().equals(".."))
250+
entryName.contains(File.separator + ".." + File.separatorChar) ||
251+
entryName.trim().endsWith(File.separator + "..") ||
252+
entryName.trim().equals(".."))
204253
{
205254
return null;
206255
}
207256

208257
// Any embedded JAR files will be extracted to the embedded directory.
209258
File embedDir = new File(m_rootDir, m_dir.getName() + EMBEDDED_DIRECTORY);
210259

211-
// Determine if the entry is an emdedded JAR file or
260+
// Determine if the entry is an embedded JAR file or
212261
// directory in the bundle JAR file. Ignore any entries
213262
// that do not exist per the spec.
214-
File file = new File(m_dir, entryName);
263+
File file = null;
264+
try {
265+
file = getFile(entryName);
266+
} catch (IOException e) {
267+
return null;
268+
}
215269
if (BundleCache.getSecureAction().isFileDirectory(file))
216270
{
217271
return new DirectoryContent(
218-
m_logger, m_configMap, m_zipFactory, m_revisionLock, m_rootDir, file);
272+
m_logger, m_configMap, m_zipFactory, m_revisionLock, m_rootDir, file);
219273
}
220274
else if (BundleCache.getSecureAction().fileExists(file)
221-
&& entryName.endsWith(".jar"))
275+
&& entryName.endsWith(".jar"))
222276
{
223277
File extractDir = new File(embedDir,
224-
(entryName.lastIndexOf('/') >= 0)
225-
? entryName.substring(0, entryName.lastIndexOf('/'))
226-
: entryName);
278+
(entryName.lastIndexOf('/') >= 0)
279+
? entryName.substring(0, entryName.lastIndexOf('/'))
280+
: entryName);
227281

228282
return new JarContent(
229-
m_logger, m_configMap, m_zipFactory, m_revisionLock,
230-
extractDir, file, null);
283+
m_logger, m_configMap, m_zipFactory, m_revisionLock,
284+
extractDir, file, null);
231285
}
232286

233287
// The entry could not be found, so return null.
234288
return null;
235289
}
236290

237-
// TODO: SECURITY - This will need to consider security.
291+
// TODO: SECURITY - This will need to consider security.
238292
public String getEntryAsNativeLibrary(String entryName)
239293
{
240294
// Return result.
@@ -245,9 +299,9 @@ public String getEntryAsNativeLibrary(String entryName)
245299
entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName;
246300

247301
if (entryName.trim().startsWith(".." + File.separatorChar) ||
248-
entryName.contains(File.separator + ".." + File.separatorChar) ||
249-
entryName.trim().endsWith(File.separator + "..") ||
250-
entryName.trim().equals(".."))
302+
entryName.contains(File.separator + ".." + File.separatorChar) ||
303+
entryName.trim().endsWith(File.separator + "..") ||
304+
entryName.trim().equals(".."))
251305
{
252306
return null;
253307
}
@@ -257,9 +311,18 @@ public String getEntryAsNativeLibrary(String entryName)
257311

258312
// The entry must exist and refer to a file, not a directory,
259313
// since we are expecting it to be a native library.
260-
File entryFile = new File(m_dir, entryName);
314+
File entryFile = null;
315+
try
316+
{
317+
entryFile = getFile(entryName);
318+
}
319+
catch (IOException e)
320+
{
321+
return null;
322+
}
323+
261324
if (BundleCache.getSecureAction().fileExists(entryFile)
262-
&& !BundleCache.getSecureAction().isFileDirectory(entryFile))
325+
&& !BundleCache.getSecureAction().isFileDirectory(entryFile))
263326
{
264327
// Extracting the embedded native library file impacts all other
265328
// existing contents for this revision, so we have to grab the
@@ -279,16 +342,16 @@ public String getEntryAsNativeLibrary(String entryName)
279342
libCount = (libCount == null) ? new Integer(0) : new Integer(libCount.intValue() + 1);
280343
m_nativeLibMap.put(entryName, libCount);
281344
File libFile = new File(
282-
libDir, libCount.toString() + File.separatorChar + entryName);
345+
libDir, libCount.toString() + File.separatorChar + entryName);
283346

284347
if (!BundleCache.getSecureAction().fileExists(libFile))
285348
{
286349
if (!BundleCache.getSecureAction().fileExists(libFile.getParentFile())
287-
&& !BundleCache.getSecureAction().mkdirs(libFile.getParentFile()))
350+
&& !BundleCache.getSecureAction().mkdirs(libFile.getParentFile()))
288351
{
289352
m_logger.log(
290-
Logger.LOG_ERROR,
291-
"Unable to create library directory.");
353+
Logger.LOG_ERROR,
354+
"Unable to create library directory.");
292355
}
293356
else
294357
{
@@ -304,7 +367,7 @@ public String getEntryAsNativeLibrary(String entryName)
304367
// Perform exec permission command on extracted library
305368
// if one is configured.
306369
String command = (String) m_configMap.get(
307-
Constants.FRAMEWORK_EXECPERMISSION);
370+
Constants.FRAMEWORK_EXECPERMISSION);
308371
if (command != null)
309372
{
310373
Properties props = new Properties();
@@ -320,8 +383,8 @@ public String getEntryAsNativeLibrary(String entryName)
320383
catch (Exception ex)
321384
{
322385
m_logger.log(
323-
Logger.LOG_ERROR,
324-
"Extracting native library.", ex);
386+
Logger.LOG_ERROR,
387+
"Extracting native library.", ex);
325388
}
326389
}
327390
}
@@ -367,7 +430,7 @@ public synchronized Object nextElement()
367430

368431
// Convert the file separator character to slashes.
369432
String abs = BundleCache.getSecureAction()
370-
.getAbsolutePath(m_children[m_counter]).replace(File.separatorChar, '/');
433+
.getAbsolutePath(m_children[m_counter]).replace(File.separatorChar, '/');
371434

372435
// Remove the leading path of the reference directory, since the
373436
// entry paths are supposed to be relative to the root.
@@ -398,7 +461,7 @@ private File[] listFilesRecursive(File dir)
398461
File[] tmp = new File[combined.length + grandchildren.length];
399462
System.arraycopy(combined, 0, tmp, 0, combined.length);
400463
System.arraycopy(
401-
grandchildren, 0, tmp, combined.length, grandchildren.length);
464+
grandchildren, 0, tmp, combined.length, grandchildren.length);
402465
combined = tmp;
403466
}
404467
}

‎framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,14 @@ public void testNoZipSlip(BundleArchive archive) throws Exception
152152

153153
public void testDirectoryReference() throws Exception
154154
{
155-
testBundle("reference:" + archiveFile.toURI().toURL(), null);
155+
BundleArchive archive = testBundle("reference:" + archiveFile.toURI().toURL(), null);
156+
String path = "../../../../../../../../../../../../../../../../..".replace("/", File.separator);
157+
assertFalse(archive.getCurrentRevision().getContent().hasEntry(path));
158+
assertFalse(archive.getCurrentRevision().getContent().isDirectory(path));
159+
assertNull(archive.getCurrentRevision().getContent().getEntryAsURL(path));
160+
assertNull(archive.getCurrentRevision().getContent().getEntryAsBytes(path + jarFile.getAbsolutePath()));
161+
assertNull(archive.getCurrentRevision().getContent().getEntryAsNativeLibrary(path + jarFile.getAbsolutePath()));
162+
assertEquals(0L, archive.getCurrentRevision().getContent().getContentTime(path + jarFile.getAbsolutePath()));
156163
}
157164

158165
public void testJarReference() throws Exception
@@ -170,7 +177,7 @@ public void testInputStream() throws Exception
170177
testBundle("bla", jarFile);
171178
}
172179

173-
private void testBundle(String location, File file) throws Exception
180+
private BundleArchive testBundle(String location, File file) throws Exception
174181
{
175182
BundleArchive archive = cache.create(1, 1, location, file != null ? new FileInputStream(file) : null, null);
176183

@@ -211,6 +218,8 @@ private void testBundle(String location, File file) throws Exception
211218
assertFalse(new File(nativeLib).isFile());
212219
assertTrue(new File(nativeLib2).isFile());
213220
assertTrue(new File(nativeLib3).isFile());
221+
222+
return archive;
214223
}
215224

216225
private void testRevision(BundleArchive archive) throws Exception

0 commit comments

Comments
 (0)