yifan-c commented on code in PR #215:
URL: https://github.com/apache/cassandra-sidecar/pull/215#discussion_r2159864967
##########
client-common/src/main/java/org/apache/cassandra/sidecar/common/ApiEndpointsV1.java:
##########
@@ -147,14 +147,10 @@ public final class ApiEndpointsV1
public static final String LIVE_MIGRATION_FILES_API =
LIVE_MIGRATION_API_PREFIX + "/files";
- public static final String LIVE_MIGRATION_CDC_RAW_DIR_PATH =
LIVE_MIGRATION_FILES_API + "/cdc_raw";
- public static final String LIVE_MIGRATION_COMMITLOG_DIR_PATH =
LIVE_MIGRATION_FILES_API + "/commitlog";
- public static final String LIVE_MIGRATION_DATA_FILE_DIR_PATH =
LIVE_MIGRATION_FILES_API + "/data";
- public static final String LIVE_MIGRATION_HINTS_DIR_PATH =
LIVE_MIGRATION_FILES_API + "/hints";
- public static final String LIVE_MIGRATION_LOCAL_SYSTEM_DATA_FILE_DIR_PATH
= LIVE_MIGRATION_FILES_API
-
+ "/local_system_data";
- public static final String LIVE_MIGRATION_SAVED_CACHES_DIR_PATH =
LIVE_MIGRATION_FILES_API + "/saved_caches";
-
+ public static final String DIR_TYPE_PARAM = "dirType";
+ public static final String DIR_INDEX_PARAM = "dirIndex";
+ public static final String LIVE_MIGRATION_FILE_TRANSFER_API =
LIVE_MIGRATION_FILES_API + "/:" + DIR_TYPE_PARAM
+ + "/:" +
DIR_INDEX_PARAM + "/*";
Review Comment:
nit: Please add a comment in the code to explain why match `/*`.
##########
server/src/main/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtil.java:
##########
@@ -54,6 +55,13 @@
public class LiveMigrationInstanceMetadataUtil
{
+ public static final String LIVE_MIGRATION_CDC_RAW_DIR_PATH =
ApiEndpointsV1.LIVE_MIGRATION_FILES_API + "/" + CDC_RAW_DIR.dirType;
+ public static final String LIVE_MIGRATION_COMMITLOG_DIR_PATH =
ApiEndpointsV1.LIVE_MIGRATION_FILES_API + "/" + COMMIT_LOG_DIR.dirType;
+ public static final String LIVE_MIGRATION_DATA_FILE_DIR_PATH =
ApiEndpointsV1.LIVE_MIGRATION_FILES_API + "/" + DATA_FIlE_DIR.dirType;
+ public static final String LIVE_MIGRATION_HINTS_DIR_PATH =
ApiEndpointsV1.LIVE_MIGRATION_FILES_API + "/" + HINTS_DIR.dirType;
+ public static final String LIVE_MIGRATION_LOCAL_SYSTEM_DATA_FILE_DIR_PATH
= ApiEndpointsV1.LIVE_MIGRATION_FILES_API
+
+ "/" + LOCAL_SYSTEM_DATA_FILE_DIR.dirType;
+ public static final String LIVE_MIGRATION_SAVED_CACHES_DIR_PATH =
ApiEndpointsV1.LIVE_MIGRATION_FILES_API + "/" + SAVED_CACHES_DIR.dirType;
Review Comment:
Why they are moved out of `ApiEndpointsV1.java`?
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamHandler.java:
##########
@@ -0,0 +1,243 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.sidecar.handlers.livemigration;
+
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.PathMatcher;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import io.vertx.ext.web.handler.HttpException;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.handlers.AbstractHandler;
+import org.apache.cassandra.sidecar.handlers.AccessProtected;
+import org.apache.cassandra.sidecar.handlers.FileStreamHandler;
+import
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil;
+import org.apache.cassandra.sidecar.utils.CassandraInputValidator;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_INDEX_PARAM;
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_TYPE_PARAM;
+import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder;
+
+/**
+ * Handler that allows Cassandra instance files to be downloaded during
LiveMigration. This handler
+ * doesn't stream the file but relies on {@link FileStreamHandler} to do so.
This handler doesn't allow
+ * using "/.." in the path to access files. This handler does not serve files
which are excluded in
+ * Live Migration configuration.
+ */
+public class LiveMigrationFileStreamHandler extends AbstractHandler<Void>
implements AccessProtected
+{
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LiveMigrationFileStreamHandler.class);
+
+ private final Map<Integer, List<PathMatcher>> fileExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final Map<Integer, List<PathMatcher>> dirExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final LiveMigrationConfiguration liveMigrationConfiguration;
+
+ @Inject
+ public LiveMigrationFileStreamHandler(InstanceMetadataFetcher
metadataFetcher,
+ ExecutorPools executorPools,
+ CassandraInputValidator validator,
+ SidecarConfiguration
sidecarConfiguration)
+ {
+ super(metadataFetcher, executorPools, validator);
+ this.liveMigrationConfiguration =
sidecarConfiguration.liveMigrationConfiguration();
+ }
+
+ @Override
+ protected Void extractParamsOrThrow(RoutingContext context)
+ {
+ String dirType = context.pathParam(DIR_TYPE_PARAM);
+ if (null == dirType || dirType.isEmpty() || null ==
LiveMigrationDirType.find(dirType))
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directory type: " + dirType);
+ }
+
+ String dirIndex = context.pathParam(DIR_INDEX_PARAM);
+ int index;
+ try
+ {
+ index = Integer.parseInt(dirIndex);
+ }
+ catch (NumberFormatException formatException)
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ }
+ if (index < 0)
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ }
+
+ // Path params are not used further, hence returning null.
+ return null;
+ }
+
+ @Override
+ protected void handleInternal(RoutingContext rc,
+ HttpServerRequest httpRequest,
+ @NotNull String host,
+ SocketAddress remoteAddress,
+ @Nullable Void request)
+ {
+ String reqPath = URLDecoder.decode(rc.request().path(),
StandardCharsets.UTF_8);
+
+ if (reqPath.contains("/../") || reqPath.endsWith("/.."))
+ {
+ LOGGER.warn("Tried to access file using relative path({}).
Rejecting the request.", reqPath);
+
rc.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end();
+ return;
+ }
+
+ InstanceMetadata instanceMeta = metadataFetcher.instance(host);
+ String normalizedPath = rc.normalizedPath();
+ String localFile;
+
+ try
+ {
+ localFile =
LiveMigrationInstanceMetadataUtil.localPath(normalizedPath, instanceMeta);
+ }
+ catch (IllegalArgumentException e)
+ {
+ LOGGER.warn("Invalid path", e);
+
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
+ return;
+ }
+
+ Path path = Paths.get(localFile);
+
+ if (!Files.exists(path))
+ {
+ LOGGER.info("File {} not found.", localFile);
+
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
Review Comment:
return early here?
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationDirType.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.sidecar.handlers.livemigration;
+
+/**
+ * Enum for holding different type of directories handled by Live Migration.
+ */
+public enum LiveMigrationDirType
+{
+ CDC_RAW_DIR("cdc_raw"),
+ COMMIT_LOG_DIR("commitlog"),
+ DATA_FIlE_DIR("data"),
+ HINTS_DIR("hints"),
+ LOCAL_SYSTEM_DATA_FILE_DIR("local_system_data"),
+ SAVED_CACHES_DIR("saved_caches");
+
+ public final String dirType;
+
+ LiveMigrationDirType(String dirType)
+ {
+ this.dirType = dirType;
+ }
+
+ public static LiveMigrationDirType find(String dirType)
+ {
+ for (LiveMigrationDirType type : values())
+ {
+ if (type.dirType.equals(dirType))
+ {
+ return type;
+ }
+ }
+ return null;
+ }
Review Comment:
You could look up much faster from a pre-defined hashmap, instead of looping
through all values.
##########
server/src/main/java/org/apache/cassandra/sidecar/acl/authorization/BasicPermissions.java:
##########
@@ -79,4 +79,5 @@ public class BasicPermissions
// Live Migration permissions
public static final Permission LIST_FILES = new
DomainAwarePermission("LIVE_MIGRATION:LIST_FILES", CLUSTER_SCOPE);
+ public static final Permission STREAM_FILES = new
DomainAwarePermission("LIVE_MIGRATION:STREAM", CLUSTER_SCOPE);
Review Comment:
In what circumstances, you want to revoke the stream permission?
According to the diagram in
[CEP-40](https://cwiki.apache.org/confluence/display/CASSANDRA/CEP-40%3A+Data+Transfer+Using+Cassandra+Sidecar+for+Live+Migrating+Instances),
list files API is invoked by sidecars, not users, right?
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamHandler.java:
##########
@@ -0,0 +1,243 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.sidecar.handlers.livemigration;
+
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.PathMatcher;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import io.vertx.ext.web.handler.HttpException;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.handlers.AbstractHandler;
+import org.apache.cassandra.sidecar.handlers.AccessProtected;
+import org.apache.cassandra.sidecar.handlers.FileStreamHandler;
+import
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil;
+import org.apache.cassandra.sidecar.utils.CassandraInputValidator;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_INDEX_PARAM;
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_TYPE_PARAM;
+import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder;
+
+/**
+ * Handler that allows Cassandra instance files to be downloaded during
LiveMigration. This handler
+ * doesn't stream the file but relies on {@link FileStreamHandler} to do so.
This handler doesn't allow
+ * using "/.." in the path to access files. This handler does not serve files
which are excluded in
+ * Live Migration configuration.
+ */
+public class LiveMigrationFileStreamHandler extends AbstractHandler<Void>
implements AccessProtected
+{
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LiveMigrationFileStreamHandler.class);
+
+ private final Map<Integer, List<PathMatcher>> fileExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final Map<Integer, List<PathMatcher>> dirExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final LiveMigrationConfiguration liveMigrationConfiguration;
+
+ @Inject
+ public LiveMigrationFileStreamHandler(InstanceMetadataFetcher
metadataFetcher,
+ ExecutorPools executorPools,
+ CassandraInputValidator validator,
+ SidecarConfiguration
sidecarConfiguration)
+ {
+ super(metadataFetcher, executorPools, validator);
+ this.liveMigrationConfiguration =
sidecarConfiguration.liveMigrationConfiguration();
+ }
+
+ @Override
+ protected Void extractParamsOrThrow(RoutingContext context)
+ {
+ String dirType = context.pathParam(DIR_TYPE_PARAM);
+ if (null == dirType || dirType.isEmpty() || null ==
LiveMigrationDirType.find(dirType))
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directory type: " + dirType);
+ }
+
+ String dirIndex = context.pathParam(DIR_INDEX_PARAM);
+ int index;
+ try
+ {
+ index = Integer.parseInt(dirIndex);
+ }
+ catch (NumberFormatException formatException)
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ }
+ if (index < 0)
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ }
+
+ // Path params are not used further, hence returning null.
+ return null;
+ }
+
+ @Override
+ protected void handleInternal(RoutingContext rc,
+ HttpServerRequest httpRequest,
+ @NotNull String host,
+ SocketAddress remoteAddress,
+ @Nullable Void request)
+ {
+ String reqPath = URLDecoder.decode(rc.request().path(),
StandardCharsets.UTF_8);
+
+ if (reqPath.contains("/../") || reqPath.endsWith("/.."))
+ {
+ LOGGER.warn("Tried to access file using relative path({}).
Rejecting the request.", reqPath);
+
rc.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end();
+ return;
+ }
+
+ InstanceMetadata instanceMeta = metadataFetcher.instance(host);
+ String normalizedPath = rc.normalizedPath();
+ String localFile;
+
+ try
+ {
+ localFile =
LiveMigrationInstanceMetadataUtil.localPath(normalizedPath, instanceMeta);
+ }
+ catch (IllegalArgumentException e)
+ {
+ LOGGER.warn("Invalid path", e);
+
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
+ return;
+ }
+
+ Path path = Paths.get(localFile);
+
+ if (!Files.exists(path))
+ {
+ LOGGER.info("File {} not found.", localFile);
Review Comment:
we use the `kv` log format consistently in the project. The log message
could be this. Please also updated the other log messages in this file.
```suggestion
LOGGER.info("Requested file is not found. file={}", localFile);
```
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamHandler.java:
##########
@@ -0,0 +1,243 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.sidecar.handlers.livemigration;
+
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.PathMatcher;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import io.vertx.ext.web.handler.HttpException;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.handlers.AbstractHandler;
+import org.apache.cassandra.sidecar.handlers.AccessProtected;
+import org.apache.cassandra.sidecar.handlers.FileStreamHandler;
+import
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil;
+import org.apache.cassandra.sidecar.utils.CassandraInputValidator;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_INDEX_PARAM;
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_TYPE_PARAM;
+import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder;
+
+/**
+ * Handler that allows Cassandra instance files to be downloaded during
LiveMigration. This handler
+ * doesn't stream the file but relies on {@link FileStreamHandler} to do so.
This handler doesn't allow
+ * using "/.." in the path to access files. This handler does not serve files
which are excluded in
+ * Live Migration configuration.
+ */
+public class LiveMigrationFileStreamHandler extends AbstractHandler<Void>
implements AccessProtected
+{
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LiveMigrationFileStreamHandler.class);
+
+ private final Map<Integer, List<PathMatcher>> fileExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final Map<Integer, List<PathMatcher>> dirExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final LiveMigrationConfiguration liveMigrationConfiguration;
+
+ @Inject
+ public LiveMigrationFileStreamHandler(InstanceMetadataFetcher
metadataFetcher,
+ ExecutorPools executorPools,
+ CassandraInputValidator validator,
+ SidecarConfiguration
sidecarConfiguration)
+ {
+ super(metadataFetcher, executorPools, validator);
+ this.liveMigrationConfiguration =
sidecarConfiguration.liveMigrationConfiguration();
+ }
+
+ @Override
+ protected Void extractParamsOrThrow(RoutingContext context)
+ {
+ String dirType = context.pathParam(DIR_TYPE_PARAM);
+ if (null == dirType || dirType.isEmpty() || null ==
LiveMigrationDirType.find(dirType))
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directory type: " + dirType);
+ }
+
+ String dirIndex = context.pathParam(DIR_INDEX_PARAM);
+ int index;
+ try
+ {
+ index = Integer.parseInt(dirIndex);
+ }
+ catch (NumberFormatException formatException)
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ }
+ if (index < 0)
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ }
+
+ // Path params are not used further, hence returning null.
+ return null;
+ }
+
+ @Override
+ protected void handleInternal(RoutingContext rc,
+ HttpServerRequest httpRequest,
+ @NotNull String host,
+ SocketAddress remoteAddress,
+ @Nullable Void request)
+ {
+ String reqPath = URLDecoder.decode(rc.request().path(),
StandardCharsets.UTF_8);
+
+ if (reqPath.contains("/../") || reqPath.endsWith("/.."))
+ {
+ LOGGER.warn("Tried to access file using relative path({}).
Rejecting the request.", reqPath);
+
rc.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end();
+ return;
+ }
+
+ InstanceMetadata instanceMeta = metadataFetcher.instance(host);
+ String normalizedPath = rc.normalizedPath();
+ String localFile;
+
+ try
+ {
+ localFile =
LiveMigrationInstanceMetadataUtil.localPath(normalizedPath, instanceMeta);
+ }
+ catch (IllegalArgumentException e)
+ {
+ LOGGER.warn("Invalid path", e);
+
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
+ return;
+ }
+
+ Path path = Paths.get(localFile);
+
+ if (!Files.exists(path))
+ {
+ LOGGER.info("File {} not found.", localFile);
+
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
+ }
+ if (Files.isDirectory(path))
+ {
+ LOGGER.info("Cannot transfer directory for request path: {}.",
reqPath);
+
rc.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end();
+ return;
+ }
+ if (isExcluded(path, instanceMeta))
+ {
+ LOGGER.debug("Requested file {} or one of its parent directories
is excluded from Live Migration.", reqPath);
+
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
+ return;
+ }
+
+ rc.put(FileStreamHandler.FILE_PATH_CONTEXT_KEY, localFile);
+ rc.next();
Review Comment:
nit: dispatch the checks to `executorPools.service()`? I am not super
concerned about the I/O access, but the `isExcluded` check seems heavy.
On success, call `rc.next()`. On failure, do nothing, as `rc` should be
ended already.
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamHandler.java:
##########
@@ -0,0 +1,243 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.sidecar.handlers.livemigration;
+
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.PathMatcher;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import io.vertx.ext.web.handler.HttpException;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.handlers.AbstractHandler;
+import org.apache.cassandra.sidecar.handlers.AccessProtected;
+import org.apache.cassandra.sidecar.handlers.FileStreamHandler;
+import
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil;
+import org.apache.cassandra.sidecar.utils.CassandraInputValidator;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_INDEX_PARAM;
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_TYPE_PARAM;
+import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder;
+
+/**
+ * Handler that allows Cassandra instance files to be downloaded during
LiveMigration. This handler
+ * doesn't stream the file but relies on {@link FileStreamHandler} to do so.
This handler doesn't allow
+ * using "/.." in the path to access files. This handler does not serve files
which are excluded in
+ * Live Migration configuration.
+ */
+public class LiveMigrationFileStreamHandler extends AbstractHandler<Void>
implements AccessProtected
+{
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LiveMigrationFileStreamHandler.class);
+
+ private final Map<Integer, List<PathMatcher>> fileExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final Map<Integer, List<PathMatcher>> dirExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final LiveMigrationConfiguration liveMigrationConfiguration;
+
+ @Inject
+ public LiveMigrationFileStreamHandler(InstanceMetadataFetcher
metadataFetcher,
+ ExecutorPools executorPools,
+ CassandraInputValidator validator,
+ SidecarConfiguration
sidecarConfiguration)
+ {
+ super(metadataFetcher, executorPools, validator);
+ this.liveMigrationConfiguration =
sidecarConfiguration.liveMigrationConfiguration();
+ }
+
+ @Override
+ protected Void extractParamsOrThrow(RoutingContext context)
+ {
+ String dirType = context.pathParam(DIR_TYPE_PARAM);
+ if (null == dirType || dirType.isEmpty() || null ==
LiveMigrationDirType.find(dirType))
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directory type: " + dirType);
+ }
+
+ String dirIndex = context.pathParam(DIR_INDEX_PARAM);
+ int index;
+ try
+ {
+ index = Integer.parseInt(dirIndex);
+ }
+ catch (NumberFormatException formatException)
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ }
+ if (index < 0)
+ {
+ throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ }
+
+ // Path params are not used further, hence returning null.
+ return null;
+ }
+
+ @Override
+ protected void handleInternal(RoutingContext rc,
+ HttpServerRequest httpRequest,
+ @NotNull String host,
+ SocketAddress remoteAddress,
+ @Nullable Void request)
+ {
+ String reqPath = URLDecoder.decode(rc.request().path(),
StandardCharsets.UTF_8);
+
+ if (reqPath.contains("/../") || reqPath.endsWith("/.."))
+ {
+ LOGGER.warn("Tried to access file using relative path({}).
Rejecting the request.", reqPath);
+
rc.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end();
+ return;
+ }
Review Comment:
In terms of validation, how is this section different from the section
below? `reqPath` is same as `normalizedPath`, right?
```
String normalizedPath = rc.normalizedPath();
String localFile;
try
{
localFile =
LiveMigrationInstanceMetadataUtil.localPath(normalizedPath, instanceMeta);
}
catch (IllegalArgumentException e)
{
LOGGER.warn("Invalid path", e);
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
return;
}
```
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamHandler.java:
##########
@@ -0,0 +1,243 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.sidecar.handlers.livemigration;
+
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.PathMatcher;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.net.SocketAddress;
+import io.vertx.ext.auth.authorization.Authorization;
+import io.vertx.ext.web.RoutingContext;
+import io.vertx.ext.web.handler.HttpException;
+import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import org.apache.cassandra.sidecar.handlers.AbstractHandler;
+import org.apache.cassandra.sidecar.handlers.AccessProtected;
+import org.apache.cassandra.sidecar.handlers.FileStreamHandler;
+import
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil;
+import org.apache.cassandra.sidecar.utils.CassandraInputValidator;
+import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_INDEX_PARAM;
+import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_TYPE_PARAM;
+import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder;
+
+/**
+ * Handler that allows Cassandra instance files to be downloaded during
LiveMigration. This handler
+ * doesn't stream the file but relies on {@link FileStreamHandler} to do so.
This handler doesn't allow
+ * using "/.." in the path to access files. This handler does not serve files
which are excluded in
+ * Live Migration configuration.
+ */
+public class LiveMigrationFileStreamHandler extends AbstractHandler<Void>
implements AccessProtected
+{
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LiveMigrationFileStreamHandler.class);
+
+ private final Map<Integer, List<PathMatcher>> fileExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final Map<Integer, List<PathMatcher>> dirExclusionsByInstanceId =
new ConcurrentHashMap<>();
+ private final LiveMigrationConfiguration liveMigrationConfiguration;
+
+ @Inject
+ public LiveMigrationFileStreamHandler(InstanceMetadataFetcher
metadataFetcher,
+ ExecutorPools executorPools,
+ CassandraInputValidator validator,
+ SidecarConfiguration
sidecarConfiguration)
+ {
+ super(metadataFetcher, executorPools, validator);
+ this.liveMigrationConfiguration =
sidecarConfiguration.liveMigrationConfiguration();
+ }
+
+ @Override
+ protected Void extractParamsOrThrow(RoutingContext context)
Review Comment:
Looks like this method is only used for some validation. Not really value
extraction.
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
