github-advanced-security[bot] commented on code in PR #18176:
URL: https://github.com/apache/druid/pull/18176#discussion_r2185245698
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -723,7 +561,101 @@
);
}
- private static class ReferenceCountingLock
+ private SegmentCacheEntry assignLocationAndMount(
+ final SegmentCacheEntry cacheEntry,
+ final SegmentLazyLoadFailCallback segmentLoadFailCallback
+ ) throws SegmentLoadingException
+ {
+ try {
+ for (StorageLocation location : locations) {
+ if (cacheEntry.checkExists(location.getPath())) {
+ if (location.isReserved(cacheEntry.id) ||
location.reserve(cacheEntry)) {
+ final SegmentCacheEntry entry =
location.getCacheEntry(cacheEntry.id);
+ entry.lazyLoadCallback = segmentLoadFailCallback;
+ entry.mount(location.getPath());
+ return entry;
+ } else {
+ cleanupCacheFiles(location.getPath(),
cacheEntry.toPotentialLocation(location.getPath()));
+ }
+ }
+ }
+ }
+ catch (IOException | SegmentLoadingException e) {
+ log.warn(e, "Failed to load segment[%s] in existing location, trying new
location", cacheEntry.id);
+ }
+ final Iterator<StorageLocation> locationsIterator =
strategy.getLocations();
+ while (locationsIterator.hasNext()) {
+ final StorageLocation location = locationsIterator.next();
+ if (location.reserve(cacheEntry)) {
+ try {
+ final SegmentCacheEntry entry =
location.getCacheEntry(cacheEntry.id);
+ entry.lazyLoadCallback = segmentLoadFailCallback;
+ entry.mount(location.getPath());
+ return entry;
+ }
+ catch (IOException | SegmentLoadingException e) {
+ log.warn(e, "Failed to load segment[%s] in location[%s], trying next
location", cacheEntry.id, location.getPath());
+ }
+ }
+ }
+ throw new SegmentLoadingException("Failed to load segment[%s] in all
locations.", cacheEntry.id);
+ }
+
+ private void cleanupCacheFiles(final File baseFile, final File cacheFile)
+ {
+ if (cacheFile.equals(baseFile)) {
+ return;
+ }
+
+ directoryWriteRemoveLock.writeLock().lock();
+ try {
+ log.info("Deleting directory[%s]", cacheFile);
+ try {
+ FileUtils.deleteDirectory(cacheFile);
+ }
+ catch (Exception e) {
+ log.error(e, "Unable to remove directory[%s]", cacheFile);
+ }
+
+ File parent = cacheFile.getParentFile();
+ if (parent != null) {
+ File[] children = parent.listFiles();
+ if (children == null || children.length == 0) {
+ cleanupCacheFiles(baseFile, parent);
+ }
+ }
+ }
+ finally {
+ directoryWriteRemoveLock.writeLock().unlock();
+ }
+ }
+
+ private static String getSegmentDir(final DataSegment segment)
+ {
+ return DataSegmentPusher.getDefaultStorageDir(segment, false);
+ }
+
+ /**
+ * check if segment data is possibly corrupted.
+ * @param dir segments cache dir
+ * @return true means segment files may be damaged.
+ */
+ private static boolean isPossiblyCorrupted(final File dir)
+ {
+ return hasStartMarker(dir);
+ }
+
+ /**
+ * If {@link #DOWNLOAD_START_MARKER_FILE_NAME} exists in the path, the
segment files might be damaged because this
+ * file is typically deleted after the segment is pulled from deep storage.
+ */
+ private static boolean hasStartMarker(final File localStorageDir)
+ {
+ final File downloadStartMarker = new File(localStorageDir.getPath(),
DOWNLOAD_START_MARKER_FILE_NAME);
+ return downloadStartMarker.exists();
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/druid/security/code-scanning/9357)
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -738,15 +670,220 @@
}
}
- @VisibleForTesting
- public ConcurrentHashMap<DataSegment, ReferenceCountingLock>
getSegmentLocks()
+ private final class SegmentCacheEntry implements CacheEntry
{
- return segmentLocks;
- }
+ private final SegmentCacheEntryIdentifier id;
+ private final DataSegment dataSegment;
+ private final String relativePathString;
+ private SegmentLazyLoadFailCallback lazyLoadCallback =
SegmentLazyLoadFailCallback.NOOP;
+ private File locationRoot;
+ private File storageDir;
+ private ReferenceCountedSegmentProvider referenceProvider;
- @VisibleForTesting
- public List<StorageLocation> getLocations()
- {
- return locations;
+ private SegmentCacheEntry(final DataSegment dataSegment)
+ {
+ this.dataSegment = dataSegment;
+ this.id = new SegmentCacheEntryIdentifier(dataSegment.getId());
+ this.relativePathString = getSegmentDir(dataSegment);
+ }
+
+ @Override
+ public SegmentCacheEntryIdentifier getId()
+ {
+ return id;
+ }
+
+ @Override
+ public long getSize()
+ {
+ return dataSegment.getSize();
+ }
+
+ @Override
+ public boolean isMounted()
+ {
+ return referenceProvider != null;
+ }
+
+ public boolean checkExists(final File location)
+ {
+ return toPotentialLocation(location).exists();
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/druid/security/code-scanning/9358)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
