Skip to content
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

snakeyaml cve fix #24099

Merged
merged 1 commit into from
Dec 3, 2024
Merged

snakeyaml cve fix #24099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 7 additions & 16 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
<dep.avro.version>1.11.4</dep.avro.version>
<dep.commons.compress.version>1.26.2</dep.commons.compress.version>
<dep.protobuf-java.version>3.25.5</dep.protobuf-java.version>
<dep.snakeyaml.version>2.0</dep.snakeyaml.version>

<!--
America/Bahia_Banderas has:
Expand Down Expand Up @@ -2057,22 +2058,6 @@
<version>1.1.3</version>
</dependency>

<dependency>
<groupId>com.facebook.presto.cassandra</groupId>
<artifactId>cassandra-server</artifactId>
<version>2.1.16-1</version>
<exclusions>
<exclusion>
<groupId>com.googlecode.json-simple</groupId>
<artifactId>json-simple</artifactId>
</exclusion>
<exclusion>
<groupId>io.netty</groupId>
<artifactId>netty-all</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>com.facebook.presto</groupId>
<artifactId>presto-delta</artifactId>
Expand Down Expand Up @@ -2218,6 +2203,12 @@
<artifactId>cassandra-driver-core</artifactId>
<version>3.11.5</version>
</dependency>

<dependency>
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
<version>${dep.snakeyaml.version}</version>
</dependency>
</dependencies>
</dependencyManagement>

Expand Down
15 changes: 13 additions & 2 deletions presto-cassandra/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@
</dependency>

<dependency>
<groupId>com.facebook.presto.cassandra</groupId>
<artifactId>cassandra-server</artifactId>
<groupId>org.testcontainers</groupId>
<artifactId>testcontainers</artifactId>
<scope>test</scope>
</dependency>

Expand Down Expand Up @@ -185,6 +185,17 @@
</dependency>
</dependencies>

<!--- dependencyManagement is to fix the Require upper bound dependencies error for slf4j -->
<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.slf4j</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this bring used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependencyManagement section is to fix the Require upper bound dependencies error for slf4j while building the cassandra module.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a little bit about why the error happens if this one was not added? Is org.testcontainers dependent on slf4j or something else?

Copy link
Contributor Author

@nishithakbhaskaran nishithakbhaskaran Nov 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @yingsu00 .

slf4j comes as a transitive dependency in org.testcontainers. If we do not add this dependencymanagement tag
the following error will come.

Rule 0: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps.
.....
.....

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0-M2:enforce (default) on project presto-cassandra: Some Enforcer rules have failed. Look above for specific messages explaining why the rule failed.

Inorder to fix this I have added the slf4j version as dependencyManagement tag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @yingsu00 .

slf4j comes as a transitive dependency in org.testcontainers. If we do not add this dependencymanagement tag the following error will come.

Rule 0: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps.
.....
.....

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0-M2:enforce (default) on project presto-cassandra: Some Enforcer rules have failed. Look above for specific messages explaining why the rule failed.

Inorder to fix this I have added the slf4j version as dependencyManagement tag.

If that's the case, can you please add a comment in the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yingsu00 Added the comment.

<artifactId>slf4j-api</artifactId>
<version>1.7.36</version>
</dependency>
</dependencies>
</dependencyManagement>

<build>
<plugins>
<plugin>
Expand Down
21 changes: 8 additions & 13 deletions presto-cassandra/src/test/java/com/facebook/presto/cassandra/CassandraQueryRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,30 +34,25 @@ private CassandraQueryRunner()

private static boolean tpchLoaded;

public static synchronized DistributedQueryRunner createCassandraQueryRunner()
yingsu00 marked this conversation as resolved.
Show resolved Hide resolved
public static DistributedQueryRunner createCassandraQueryRunner(CassandraServer server)
throws Exception
{
EmbeddedCassandra.start();

DistributedQueryRunner queryRunner = new DistributedQueryRunner(createCassandraSession("tpch"), 4);

queryRunner.installPlugin(new TpchPlugin());
queryRunner.createCatalog("tpch", "tpch");

queryRunner.installPlugin(new CassandraPlugin());
queryRunner.createCatalog("cassandra", "cassandra", ImmutableMap.of(
"cassandra.contact-points", EmbeddedCassandra.getHost(),
"cassandra.native-protocol-port", Integer.toString(EmbeddedCassandra.getPort()),
"cassandra.contact-points", server.getHost(),
"cassandra.native-protocol-port", Integer.toString(server.getPort()),
"cassandra.allow-drop-table", "true"));

if (!tpchLoaded) {
createKeyspace(EmbeddedCassandra.getSession(), "tpch");
List<TpchTable<?>> tables = TpchTable.getTables();
copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, createCassandraSession("tpch"), tables);
for (TpchTable table : tables) {
EmbeddedCassandra.refreshSizeEstimates("tpch", table.getTableName());
}
tpchLoaded = true;
createKeyspace(server.getSession(), "tpch");
List<TpchTable<?>> tables = TpchTable.getTables();
copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, createCassandraSession("tpch"), tables);
for (TpchTable<?> table : tables) {
server.refreshSizeEstimates("tpch", table.getTableName());
}

return queryRunner;
Expand Down
93 changes: 34 additions & 59 deletions ...k/presto/cassandra/EmbeddedCassandra.java → ...ook/presto/cassandra/CassandraServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,18 @@
import com.google.common.collect.ImmutableList;
import com.google.common.io.Resources;
import io.airlift.units.Duration;
import org.apache.cassandra.service.CassandraDaemon;

import javax.management.ObjectName;
import org.testcontainers.containers.GenericContainer;

import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.lang.management.ManagementFactory;
import java.net.InetSocketAddress;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.concurrent.TimeoutException;

import static com.datastax.driver.core.ProtocolVersion.V3;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.io.Files.createTempDir;
import static com.google.common.io.Files.write;
import static com.google.common.io.Resources.getResource;
Expand All @@ -45,43 +42,38 @@
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.testcontainers.utility.MountableFile.forHostPath;
import static org.testng.Assert.assertEquals;

public final class EmbeddedCassandra
public class CassandraServer
implements Closeable

{
private static Logger log = Logger.get(EmbeddedCassandra.class);
private static Logger log = Logger.get(CassandraServer.class);

private static final String HOST = "127.0.0.1";
private static final int PORT = 9142;

private static final Duration REFRESH_SIZE_ESTIMATES_TIMEOUT = new Duration(1, MINUTES);

private static CassandraSession session;
private static boolean initialized;
private final GenericContainer<?> dockerContainer;

private EmbeddedCassandra() {}
private final CassandraSession session;

public static synchronized void start()
public CassandraServer()
throws Exception
{
if (initialized) {
return;
}

log.info("Starting cassandra...");

System.setProperty("cassandra.config", "file:" + prepareCassandraYaml());
System.setProperty("cassandra-foreground", "true");
System.setProperty("cassandra.native.epoll.enabled", "false");

CassandraDaemon cassandraDaemon = new CassandraDaemon();
cassandraDaemon.activate();
this.dockerContainer = new GenericContainer<>("cassandra:2.1.16")
.withExposedPorts(PORT)
.withCopyFileToContainer(forHostPath(prepareCassandraYaml()), "/etc/cassandra/cassandra.yaml");
this.dockerContainer.start();

Cluster.Builder clusterBuilder = Cluster.builder()
.withProtocolVersion(V3)
.withClusterName("TestCluster")
.addContactPointsWithPorts(ImmutableList.of(
new InetSocketAddress(HOST, PORT)))
new InetSocketAddress(this.dockerContainer.getContainerIpAddress(), this.dockerContainer.getMappedPort(PORT))))
.withMaxSchemaAgreementWaitSeconds(30);

ReopeningCluster cluster = new ReopeningCluster(clusterBuilder::build);
Expand All @@ -96,12 +88,11 @@ public static synchronized void start()
}
catch (RuntimeException e) {
cluster.close();
cassandraDaemon.deactivate();
this.dockerContainer.stop();
throw e;
}

EmbeddedCassandra.session = session;
initialized = true;
this.session = session;
}

private static String prepareCassandraYaml()
Expand All @@ -115,35 +106,25 @@ private static String prepareCassandraYaml()
Path dataDir = tmpDirPath.resolve("data");
Files.createDirectory(dataDir);

String modified = original.replaceAll("\\$\\{data_directory\\}", dataDir.toAbsolutePath().toString());

Path yamlLocation = tmpDirPath.resolve("cu-cassandra.yaml");
write(modified, yamlLocation.toFile(), UTF_8);
write(original, yamlLocation.toFile(), UTF_8);

return yamlLocation.toAbsolutePath().toString();
}

public static synchronized CassandraSession getSession()
public CassandraSession getSession()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For line 109:

Since you removed data_directory from cu-cassandra.yaml, this is no longer needed. However we need to make sure the path you specified always exist and won't cause error on different platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I have removed the line. Also the path is inside cassandra docker container and it will be independent of the platforms, so I believe this won't cause issues. I have updated the cherrypick id in the description of the PR.

{
checkIsInitialized();
return requireNonNull(session, "cluster is null");
}

public static synchronized String getHost()
public String getHost()
{
checkIsInitialized();
return HOST;
return dockerContainer.getContainerIpAddress();
}

public static synchronized int getPort()
public int getPort()
{
checkIsInitialized();
return PORT;
}

private static void checkIsInitialized()
{
checkState(initialized, "EmbeddedCassandra must be started with #start() method before retrieving the cluster retrieval");
return dockerContainer.getMappedPort(PORT);
}

private static void checkConnectivity(CassandraSession session)
Expand All @@ -155,7 +136,7 @@ private static void checkConnectivity(CassandraSession session)
log.info("Cassandra version: %s", version);
}

public static void refreshSizeEstimates(String keyspace, String table)
public void refreshSizeEstimates(String keyspace, String table)
throws Exception
{
long deadline = System.nanoTime() + REFRESH_SIZE_ESTIMATES_TIMEOUT.roundTo(NANOSECONDS);
Expand All @@ -173,27 +154,21 @@ public static void refreshSizeEstimates(String keyspace, String table)
throw new TimeoutException(format("Attempting to refresh size estimates for table %s.%s has timed out after %s", keyspace, table, REFRESH_SIZE_ESTIMATES_TIMEOUT));
}

private static void flushTable(String keyspace, String table)
private void flushTable(String keyspace, String table)
throws Exception
{
ManagementFactory
.getPlatformMBeanServer()
.invoke(
new ObjectName("org.apache.cassandra.db:type=StorageService"),
"forceKeyspaceFlush",
new Object[] {keyspace, new String[] {table}},
new String[] {"java.lang.String", "[Ljava.lang.String;"});
dockerContainer.execInContainer("nodetool", "flush", keyspace, table);
}

private static void refreshSizeEstimates()
private void refreshSizeEstimates()
throws Exception
{
ManagementFactory
.getPlatformMBeanServer()
.invoke(
new ObjectName("org.apache.cassandra.db:type=StorageService"),
"refreshSizeEstimates",
new Object[] {},
new String[] {});
dockerContainer.execInContainer("nodetool", "refreshsizeestimates");
}

@Override
public void close()
{
dockerContainer.close();
}
}
26 changes: 13 additions & 13 deletions presto-cassandra/src/test/java/com/facebook/presto/cassandra/CassandraTestingUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ public static void createTableClusteringKeys(CassandraSession session, SchemaTab

public static void insertIntoTableClusteringKeys(CassandraSession session, SchemaTableName table, int rowsCount)
{
for (Integer rowNumber = 1; rowNumber <= rowsCount; rowNumber++) {
for (int rowNumber = 1; rowNumber <= rowsCount; rowNumber++) {
Insert insert = QueryBuilder.insertInto(table.getSchemaName(), table.getTableName())
.value("key", "key_" + rowNumber.toString())
.value("key", "key_" + rowNumber)
.value("clust_one", "clust_one")
.value("clust_two", "clust_two_" + rowNumber.toString())
.value("clust_three", "clust_three_" + rowNumber.toString());
.value("clust_two", "clust_two_" + rowNumber)
.value("clust_three", "clust_three_" + rowNumber);
session.execute(insert);
}
assertEquals(session.execute("SELECT COUNT(*) FROM " + table).all().get(0).getLong(0), rowsCount);
Expand All @@ -103,13 +103,13 @@ public static void createTableMultiPartitionClusteringKeys(CassandraSession sess

public static void insertIntoTableMultiPartitionClusteringKeys(CassandraSession session, SchemaTableName table)
{
for (Integer rowNumber = 1; rowNumber < 10; rowNumber++) {
for (int rowNumber = 1; rowNumber < 10; rowNumber++) {
Insert insert = QueryBuilder.insertInto(table.getSchemaName(), table.getTableName())
.value("partition_one", "partition_one_" + rowNumber.toString())
.value("partition_two", "partition_two_" + rowNumber.toString())
.value("partition_one", "partition_one_" + rowNumber)
.value("partition_two", "partition_two_" + rowNumber)
.value("clust_one", "clust_one")
.value("clust_two", "clust_two_" + rowNumber.toString())
.value("clust_three", "clust_three_" + rowNumber.toString());
.value("clust_two", "clust_two_" + rowNumber)
.value("clust_three", "clust_three_" + rowNumber);
session.execute(insert);
}
assertEquals(session.execute("SELECT COUNT(*) FROM " + table).all().get(0).getLong(0), 9);
Expand All @@ -131,7 +131,7 @@ public static void createTableClusteringKeysInequality(CassandraSession session,

public static void insertIntoTableClusteringKeysInequality(CassandraSession session, SchemaTableName table, Date date, int rowsCount)
{
for (Integer rowNumber = 1; rowNumber <= rowsCount; rowNumber++) {
for (int rowNumber = 1; rowNumber <= rowsCount; rowNumber++) {
Insert insert = QueryBuilder.insertInto(table.getSchemaName(), table.getTableName())
.value("key", "key_1")
.value("clust_one", "clust_one")
Expand Down Expand Up @@ -222,12 +222,12 @@ public static void createTableAllTypesPartitionKey(CassandraSession session, Sch

private static void insertTestData(CassandraSession session, SchemaTableName table, Date date, int rowsCount)
{
for (Integer rowNumber = 1; rowNumber <= rowsCount; rowNumber++) {
for (int rowNumber = 1; rowNumber <= rowsCount; rowNumber++) {
Insert insert = QueryBuilder.insertInto(table.getSchemaName(), table.getTableName())
.value("key", "key " + rowNumber.toString())
.value("key", "key " + rowNumber)
.value("typeuuid", UUID.fromString(String.format("00000000-0000-0000-0000-%012d", rowNumber)))
.value("typeinteger", rowNumber)
.value("typelong", rowNumber.longValue() + 1000)
.value("typelong", rowNumber + 1000)
.value("typebytes", ByteBuffer.wrap(Ints.toByteArray(rowNumber)).asReadOnlyBuffer())
.value("typetimestamp", date)
.value("typeansi", "ansi " + rowNumber)
Expand Down
Loading
Loading