Skip to content

fix: secure temporary files and resource lookups #19822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FrankChen021 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: secure temporary files and resource lookups #19822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0bae8de
Fix CodeQL temporary file and resource lookup warnings
FrankChen021 Jul 30, 2026
7c03364
test: validate resource lookups before file output
FrankChen021 Jul 30, 2026
5111432
Keep mapped spatial test indexes alive
FrankChen021 Aug 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions benchmarks/src/test/java/org/apache/druid/benchmark/FrontCodedIndexedBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,13 @@ public void createIndex() throws IOException
frontCodedIndexedWriterIncrementalBuckets.write(StringUtils.toUtf8Nullable(next));
}
smooshDirFrontCoded = FileUtils.createTempDir();
fileFrontCoded = File.createTempFile("frontCodedIndexedBenchmark", "meta");
fileFrontCoded = new File(smooshDirFrontCoded, "meta");

smooshDirGeneric = FileUtils.createTempDir();
fileGeneric = File.createTempFile("genericIndexedBenchmark", "meta");
fileGeneric = new File(smooshDirGeneric, "meta");

smooshDirFrontCodedIncrementalBuckets = FileUtils.createTempDir();
fileFrontCodedIncrementalBuckets = File.createTempFile("frontCodedIndexedBenchmarkv1Buckets", "meta");
fileFrontCodedIncrementalBuckets = new File(smooshDirFrontCodedIncrementalBuckets, "meta");

EncodingSizeProfiler.encodedSize = (int) ("generic".equals(indexType)
? genericIndexedWriter.getSerializedSize()
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/src/test/java/org/apache/druid/benchmark/GenericIndexedBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public void createGenericIndexed() throws IOException
genericIndexedWriter.write(element.array());
}
smooshDir = FileUtils.createTempDir();
file = File.createTempFile("genericIndexedBenchmark", "meta");
file = new File(smooshDir, "meta");

try (FileChannel fileChannel =
FileChannel.open(file.toPath(), StandardOpenOption.CREATE, StandardOpenOption.WRITE);
Expand Down
6 changes: 2 additions & 4 deletions benchmarks/src/test/java/org/apache/druid/benchmark/indexing/IndexMergeBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,7 @@ public void setup() throws IOException
@OutputTimeUnit(TimeUnit.MICROSECONDS)
public void mergeV9(Blackhole blackhole) throws Exception
{
File tmpFile = File.createTempFile("IndexMergeBenchmark-MERGEDFILE-V9-" + System.currentTimeMillis(), ".TEMPFILE");
tmpFile.delete();
FileUtils.mkdirp(tmpFile);
final File tmpFile = FileUtils.createTempDir("IndexMergeBenchmark-MERGEDFILE-V9-");
try {
log.info(tmpFile.getAbsolutePath() + " isFile: " + tmpFile.isFile() + " isDir:" + tmpFile.isDirectory());

Expand All @@ -167,7 +165,7 @@ public void mergeV9(Blackhole blackhole) throws Exception
blackhole.consume(mergedFile);
}
finally {
tmpFile.delete();
FileUtils.deleteDirectory(tmpFile);
}
}

Expand Down
39 changes: 21 additions & 18 deletions ...un-oss-extensions/src/main/java/org/apache/druid/storage/aliyun/OssDataSegmentPusher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.nio.file.Files;
import java.util.Map;

public class OssDataSegmentPusher implements DataSegmentPusher
Expand Down Expand Up @@ -64,27 +65,29 @@ public DataSegment pushToPath(File indexFilesDir, DataSegment inSegment, String
final String path = OssUtils.constructSegmentPath(config.getPrefix(), storageDirSuffix);
log.debug("Copying segment[%s] to OSS at location[%s]", inSegment.getId(), path);

final File zipOutFile = File.createTempFile("druid", "index.zip");
final long indexSize = CompressionUtils.zip(indexFilesDir, zipOutFile);
final File zipOutFile = Files.createTempFile("druid", "index.zip").toFile();
try {
final long indexSize = CompressionUtils.zip(indexFilesDir, zipOutFile);

final DataSegment outSegment = inSegment.withSize(indexSize)
.withLoadSpec(makeLoadSpec(config.getBucket(), path))
.withBinaryVersion(SegmentUtils.getVersionFromDir(indexFilesDir));
final DataSegment outSegment = inSegment.withSize(indexSize)
.withLoadSpec(makeLoadSpec(config.getBucket(), path))
.withBinaryVersion(SegmentUtils.getVersionFromDir(indexFilesDir));

try {
return OssUtils.retry(
() -> {
OssUtils.uploadFileIfPossible(client, config.getBucket(), path, zipOutFile);
try {
return OssUtils.retry(
() -> {
OssUtils.uploadFileIfPossible(client, config.getBucket(), path, zipOutFile);

return outSegment;
}
);
}
catch (OSSException e) {
throw new IOException(e);
}
catch (Exception e) {
throw new RuntimeException(e);
return outSegment;
}
);
}
catch (OSSException e) {
throw new IOException(e);
}
catch (Exception e) {
throw new RuntimeException(e);
}
}
finally {
log.debug("Deleting temporary cached index.zip");
Expand Down
41 changes: 26 additions & 15 deletions .../apache/druid/emitter/ambari/metrics/WhiteListBasedDruidToTimelineEventConverterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,24 @@
import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
import org.apache.hadoop.metrics2.sink.timeline.TimelineMetric;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.Objects;

@RunWith(JUnitParamsRunner.class)
public class WhiteListBasedDruidToTimelineEventConverterTest
{
@Rule
public final TemporaryFolder temporaryFolder = new TemporaryFolder();

private final String prefix = "druid";
private final WhiteListBasedDruidToTimelineEventConverter defaultWhiteListBasedDruidToTimelineEventConverter =
new WhiteListBasedDruidToTimelineEventConverter(prefix, "druid", null, new DefaultObjectMapper());
Expand Down Expand Up @@ -93,29 +100,33 @@ public void testGetName(ServiceMetricEvent serviceMetricEvent, String expectedPa
@Test
public void testWhiteListedStringArrayDimension() throws IOException
{
File mapFile = File.createTempFile("testing-" + System.nanoTime(), ".json");
mapFile.deleteOnExit();
final File mapFile = temporaryFolder.newFile("whiteList.json");

try (OutputStream outputStream = new FileOutputStream(mapFile)) {
IOUtils.copyLarge(
getClass().getResourceAsStream("/testWhiteListedStringArrayDimension.json"),
outputStream
);
try (
final InputStream inputStream = Objects.requireNonNull(
WhiteListBasedDruidToTimelineEventConverterTest.class
.getResourceAsStream("/testWhiteListedStringArrayDimension.json"),
"Missing test resource: /testWhiteListedStringArrayDimension.json"
);
final OutputStream outputStream = new FileOutputStream(mapFile)
) {
IOUtils.copyLarge(inputStream, outputStream);
}

WhiteListBasedDruidToTimelineEventConverter converter = new WhiteListBasedDruidToTimelineEventConverter(
prefix,
"druid",
mapFile.getAbsolutePath(),
new DefaultObjectMapper()
);
final WhiteListBasedDruidToTimelineEventConverter converter =
new WhiteListBasedDruidToTimelineEventConverter(
prefix,
"druid",
mapFile.getAbsolutePath(),
new DefaultObjectMapper()
);

ServiceMetricEvent event = new ServiceMetricEvent.Builder()
final ServiceMetricEvent event = new ServiceMetricEvent.Builder()
.setDimension("gcName", new String[] {"g1"})
.setMetric("jvm/gc/cpu", 10)
.build(serviceName, hostname);

TimelineMetric metric = converter.druidEventToTimelineMetric(event);
final TimelineMetric metric = converter.druidEventToTimelineMetric(event);

Assert.assertNotNull(metric);
Assert.assertEquals(defaultNamespace + ".g1.jvm/gc/cpu", metric.getMetricName());
Expand Down
56 changes: 29 additions & 27 deletions ...-storage/src/main/java/org/apache/druid/storage/cassandra/CassandraDataSegmentPusher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,40 +70,42 @@ public DataSegment push(final File indexFilesDir, DataSegment segment, final boo
@Override
public DataSegment pushToPath(File indexFilesDir, DataSegment segment, String storageDirSuffix) throws IOException
{
String key = JOINER.join(
final String key = JOINER.join(
config.getKeyspace().isEmpty() ? null : config.getKeyspace(),
storageDirSuffix
);
);
// Create index
final File compressedIndexFile = File.createTempFile("druid", "index.zip");
long indexSize = CompressionUtils.zip(indexFilesDir, compressedIndexFile);
log.info("Wrote compressed file [%s] to [%s]", compressedIndexFile.getAbsolutePath(), key);
final File compressedIndexFile = Files.createTempFile("druid", "index.zip").toFile();
try {
final long indexSize = CompressionUtils.zip(indexFilesDir, compressedIndexFile);
log.info("Wrote compressed file [%s] to [%s]", compressedIndexFile.getAbsolutePath(), key);

final int version = SegmentUtils.getVersionFromDir(indexFilesDir);

int version = SegmentUtils.getVersionFromDir(indexFilesDir);
try (final InputStream fileStream = Files.newInputStream(compressedIndexFile.toPath())) {
final long start = System.currentTimeMillis();
ChunkedStorage.newWriter(indexStorage, key, fileStream)
.withConcurrencyLevel(CONCURRENCY).call();
final byte[] json = jsonMapper.writeValueAsBytes(segment);
final MutationBatch mutation = this.keyspace.prepareMutationBatch();
mutation.withRow(descriptorStorage, key)
.putColumn("lastmodified", System.currentTimeMillis(), null)
.putColumn("descriptor", json, null);
mutation.execute();
log.info("Wrote index to C* in [%s] ms", System.currentTimeMillis() - start);
}
catch (Exception e) {
throw new IOException(e);
}

try (final InputStream fileStream = Files.newInputStream(compressedIndexFile.toPath())) {
long start = System.currentTimeMillis();
ChunkedStorage.newWriter(indexStorage, key, fileStream)
.withConcurrencyLevel(CONCURRENCY).call();
byte[] json = jsonMapper.writeValueAsBytes(segment);
MutationBatch mutation = this.keyspace.prepareMutationBatch();
mutation.withRow(descriptorStorage, key)
.putColumn("lastmodified", System.currentTimeMillis(), null)
.putColumn("descriptor", json, null);
mutation.execute();
log.info("Wrote index to C* in [%s] ms", System.currentTimeMillis() - start);
return segment.withSize(indexSize)
.withLoadSpec(ImmutableMap.of("type", "c*", "key", key))
.withBinaryVersion(version);
}
catch (Exception e) {
throw new IOException(e);
finally {
log.info("Deleting zipped index File[%s]", compressedIndexFile);
compressedIndexFile.delete();
}

segment = segment.withSize(indexSize)
.withLoadSpec(ImmutableMap.of("type", "c*", "key", key))
.withBinaryVersion(version);

log.info("Deleting zipped index File[%s]", compressedIndexFile);
compressedIndexFile.delete();
return segment;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions ...nsions/src/main/java/org/apache/druid/storage/cloudfiles/CloudFilesDataSegmentPusher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ public DataSegment pushToPath(File indexFilesDir, DataSegment inSegment, String
File zipOutFile = null;

try {
final File descFile = descriptorFile = File.createTempFile("descriptor", ".json");
final File outFile = zipOutFile = File.createTempFile("druid", "index.zip");
final File descFile = descriptorFile = Files.createTempFile("descriptor", ".json").toFile();
final File outFile = zipOutFile = Files.createTempFile("druid", "index.zip").toFile();

final long indexSize = CompressionUtils.zip(indexFilesDir, zipOutFile);

Expand Down
28 changes: 18 additions & 10 deletions .../druid/compressedbigdecimal/aggregator/CompressedBigDecimalAggregatorGroupByTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.math.BigDecimal;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.TimeZone;


Expand Down Expand Up @@ -95,16 +97,22 @@ public static void setupClass()
@Test
public void testIngestAndGroupByAllQuery() throws IOException, Exception
{
Sequence<ResultRow> seq = helper.createIndexAndRunQueryOnSegment(
this.getClass().getResourceAsStream("/" + "bd_test_data.csv"),
CompressedBigDecimalAggregatorTimeseriesTestBase.SCHEMA,
CompressedBigDecimalAggregatorTimeseriesTestBase.FORMAT,
cbdGroupByQueryConfig.getIngestionAggregators(),
0,
Granularities.NONE,
5,
cbdGroupByQueryConfig.getQuery()
);
final Sequence<ResultRow> seq;
try (InputStream inputStream = Objects.requireNonNull(
CompressedBigDecimalAggregatorGroupByTestBase.class.getResourceAsStream("/bd_test_data.csv"),
"Missing resource /bd_test_data.csv"
)) {
seq = helper.createIndexAndRunQueryOnSegment(
inputStream,
CompressedBigDecimalAggregatorTimeseriesTestBase.SCHEMA,
CompressedBigDecimalAggregatorTimeseriesTestBase.FORMAT,
cbdGroupByQueryConfig.getIngestionAggregators(),
0,
Granularities.NONE,
5,
cbdGroupByQueryConfig.getQuery()
);
}

List<ResultRow> results = seq.toList();
Assert.assertThat(results, IsCollectionWithSize.hasSize(1));
Expand Down
51 changes: 37 additions & 14 deletions ...uid/compressedbigdecimal/aggregator/CompressedBigDecimalAggregatorTimeseriesTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,15 @@
import org.junit.rules.TemporaryFolder;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.math.BigDecimal;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.TimeZone;

import static org.hamcrest.collection.IsMapContaining.hasEntry;
Expand Down Expand Up @@ -126,16 +131,22 @@ protected void testIngestAndTimeseriesQueryHelper(
String expected
) throws Exception
{
Sequence seq = helper.createIndexAndRunQueryOnSegment(
this.getClass().getResourceAsStream("/" + "bd_test_data.csv"),
SCHEMA,
FORMAT,
ingestionAggregators,
0,
Granularities.NONE,
5,
query
);
final Sequence seq;
try (InputStream inputStream = Objects.requireNonNull(
CompressedBigDecimalAggregatorTimeseriesTestBase.class.getResourceAsStream("/bd_test_data.csv"),
"Missing resource /bd_test_data.csv"
)) {
seq = helper.createIndexAndRunQueryOnSegment(
inputStream,
SCHEMA,
FORMAT,
ingestionAggregators,
0,
Granularities.NONE,
5,
query
);
}

TimeseriesResultValue result = ((Result<TimeseriesResultValue>) Iterables.getOnlyElement(seq.toList())).getValue();
Map<String, Object> event = result.getBaseObject();
Expand Down Expand Up @@ -164,9 +175,9 @@ protected void testIngestMultipleSegmentsAndTimeseriesQueryHelper(
String expected
) throws Exception
{
File segmentDir1 = tempFolder.newFolder();
final File segmentDir1 = tempFolder.newFolder();
helper.createIndex(
new File(this.getClass().getResource("/" + "bd_test_data.csv").getFile()),
copyResourceToTemporaryFile("/bd_test_data.csv"),
SCHEMA,
FORMAT,
ingestionAggregators,
Expand All @@ -175,9 +186,9 @@ protected void testIngestMultipleSegmentsAndTimeseriesQueryHelper(
Granularities.NONE,
5
);
File segmentDir2 = tempFolder.newFolder();
final File segmentDir2 = tempFolder.newFolder();
helper.createIndex(
new File(this.getClass().getResource("/" + "bd_test_zero_data.csv").getFile()),
copyResourceToTemporaryFile("/bd_test_zero_data.csv"),
SCHEMA,
FORMAT,
ingestionAggregators,
Expand Down Expand Up @@ -205,4 +216,16 @@ protected void testIngestMultipleSegmentsAndTimeseriesQueryHelper(
);

}

private File copyResourceToTemporaryFile(final String resource) throws IOException
{
final File resourceFile = tempFolder.newFile();
try (InputStream inputStream = Objects.requireNonNull(
CompressedBigDecimalAggregatorTimeseriesTestBase.class.getResourceAsStream(resource),
"Missing resource " + resource
)) {
Files.copy(inputStream, resourceFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
}
return resourceFile;
}
}
Loading
Loading