Skip to content

Commit ce2719a

Browse files
committed
Addressed review comments.
1 parent f70c2c1 commit ce2719a

6 files changed

Lines changed: 62 additions & 5 deletions

File tree

server/src/main/java/org/apache/cassandra/sidecar/exceptions/LiveMigrationExceptions.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,22 @@ public LiveMigrationTaskNotFoundException(String message)
7272
}
7373
}
7474

75+
/**
76+
* Thrown when a live-migration file URL is well-formed but matches no configured directory prefix
77+
* on this instance, i.e. it addresses no resource here. Extends {@link IllegalArgumentException} so
78+
* that destination-side callers - which map file URLs obtained from the source's file-list API onto
79+
* local paths - keep treating an unmatched prefix (a source/destination directory-configuration
80+
* mismatch) as a bad argument, while the source-side request handler can catch this specific type to
81+
* distinguish "no such resource" (HTTP 404) from a malformed URL (HTTP 400).
82+
*/
83+
public static class UnknownMigrationPrefixException extends IllegalArgumentException
84+
{
85+
public UnknownMigrationPrefixException(String message)
86+
{
87+
super(message);
88+
}
89+
}
90+
7591
/**
7692
* Exception thrown when file verification fails during live migration.
7793
*/

server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileResolveHandler.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
4848
import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration;
4949
import org.apache.cassandra.sidecar.config.SidecarConfiguration;
50+
import org.apache.cassandra.sidecar.exceptions.LiveMigrationExceptions.UnknownMigrationPrefixException;
5051
import org.apache.cassandra.sidecar.handlers.AbstractHandler;
5152
import org.apache.cassandra.sidecar.handlers.AccessProtected;
5253
import org.apache.cassandra.sidecar.handlers.FileStreamHandler;
@@ -151,8 +152,16 @@ protected void handleInternal(RoutingContext rc,
151152
{
152153
resolved = LiveMigrationInstanceMetadataUtil.resolveLexically(normalizedPath, instanceMeta);
153154
}
155+
catch (UnknownMigrationPrefixException e)
156+
{
157+
// The URL is well-formed but matches no configured live-migration directory on this
158+
// instance, so it addresses no resource here - report it as not found.
159+
rc.fail(wrapHttpException(HttpResponseStatus.NOT_FOUND, e.getMessage(), e));
160+
return;
161+
}
154162
catch (IllegalArgumentException e)
155163
{
164+
// URL must have been malformed, report it as bad request
156165
rc.fail(wrapHttpException(HttpResponseStatus.BAD_REQUEST, e.getMessage(), e));
157166
return;
158167
}

server/src/main/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtil.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939

4040
import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
4141
import org.apache.cassandra.sidecar.common.ApiEndpointsV1;
42+
import org.apache.cassandra.sidecar.exceptions.LiveMigrationExceptions.UnknownMigrationPrefixException;
4243
import org.apache.cassandra.sidecar.handlers.livemigration.LiveMigrationDirType;
4344
import org.jetbrains.annotations.NotNull;
4445

@@ -306,6 +307,20 @@ private static Path canonicalBaseDir(Path baseDir) throws IOException
306307
return existing != null ? existing : canonical;
307308
}
308309

310+
/**
311+
* Lexically resolves a live migration file download URL to a {@link ResolvedPath}. Performs only
312+
* string-level validation; the filesystem is not touched and the file is not required to exist.
313+
*
314+
* @param fileUrl Live migration file download URL
315+
* @param metadata Cassandra instance metadata
316+
* @return the lexically-resolved {@link ResolvedPath}
317+
* @throws IllegalArgumentException if the URL is malformed - it contains a relative traversal
318+
* segment ({@code /../}) or lexically escapes the configured base directory
319+
* @throws UnknownMigrationPrefixException if the URL does not match any configured live-migration directory
320+
* prefix; the URL is well-formed but addresses no resource on this
321+
* instance. This is a subtype of {@link IllegalArgumentException}, so
322+
* callers that only care about "bad URL" need not distinguish it
323+
*/
309324
public static ResolvedPath resolveLexically(@NotNull String fileUrl,
310325
@NotNull InstanceMetadata metadata)
311326
{
@@ -341,7 +356,7 @@ public static ResolvedPath resolveLexically(@NotNull String fileUrl,
341356
}
342357

343358
LOGGER.warn("File url {} does not match any configured live-migration directory prefix.", fileUrl);
344-
throw new IllegalArgumentException("File url " + fileUrl + " is unknown.");
359+
throw new UnknownMigrationPrefixException("File url " + fileUrl + " is unknown.");
345360
}
346361

347362
/**

server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileDigestHandlerTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ public void testRouteFailsForInvalidDir(VertxTestContext context) throws IOExcep
185185
createFile(dummyText, firstInstanceMeta.commitlogDir() + filePath);
186186

187187
String testRoute = LIVE_MIGRATION_FILES_ROUTE + "/commitlog/1" + filePath + "?digestAlgorithm=xxhash32";
188-
shouldFail(context, testRoute, FIRST_INSTANCE_IP, SECOND_INSTANCE_IP, FIRST_INSTANCE_IP, 400);
188+
shouldFail(context, testRoute, FIRST_INSTANCE_IP, SECOND_INSTANCE_IP, FIRST_INSTANCE_IP, 404);
189189
}
190190

191191
@Test

server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,10 +316,10 @@ public void testRouteHavingDirIndexThatDoesNotExist(VertxTestContext context) th
316316
createFile(DUMMY_CONTENT, firstInstanceDataDirs.get(0) + filePath);
317317

318318
// dirIndex 100 is greater than the configured data dir count, so the URL doesn't match
319-
// any configured prefix — resolveLexically rejects it as a malformed URL (400), not a
320-
// missing file (404). The request can never resolve to a file on this server.
319+
// any configured prefix - reports it as a missing resource (404), since
320+
// the URL is well-formed but addresses no directory configured on this instance.
321321
shouldThrowError(context, LIVE_MIGRATION_DATA_FILE_DIR_PATH + "/100" + filePath,
322-
FIRST_INSTANCE_IP, THIRD_INSTANCE_IP, FIRST_INSTANCE_IP, 400);
322+
FIRST_INSTANCE_IP, THIRD_INSTANCE_IP, FIRST_INSTANCE_IP, 404);
323323
}
324324

325325
@Test

server/src/test/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtilTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.junit.jupiter.api.io.TempDir;
3737

3838
import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
39+
import org.apache.cassandra.sidecar.exceptions.LiveMigrationExceptions.UnknownMigrationPrefixException;
3940
import org.mockito.Mockito;
4041

4142
import static org.apache.cassandra.sidecar.handlers.livemigration.InstanceMetadataTestUtil.LIVE_MIGRATION_CDC_RAW_DIR_PATH;
@@ -411,6 +412,22 @@ public void testRealPathRejectsLexicalTraversal()
411412
LIVE_MIGRATION_DATA_FILE_DIR_PATH + "/0/../../etc/passwd", instanceMetadata));
412413
}
413414

415+
@Test
416+
public void testResolveLexicallyThrowsForUnknownPrefix()
417+
{
418+
// A well-formed URL whose prefix matches no configured directory is reported via
419+
// UnknownMigrationPrefixException. It is a subtype of IllegalArgumentException (so destination-side
420+
// localPath callers keep treating it as a bad argument), while the source-side handler catches this
421+
// specific type to answer 404 - "no such resource" - instead of 400 for a malformed URL.
422+
String cassandraHomeDir = tempDir.resolve("unknownPrefix").toString();
423+
InstanceMetadata instanceMetadata = getInstanceMetadata(cassandraHomeDir);
424+
when(instanceMetadata.cdcDir()).thenReturn(null);
425+
426+
assertThatExceptionOfType(UnknownMigrationPrefixException.class)
427+
.isThrownBy(() -> resolveLexically(LIVE_MIGRATION_CDC_RAW_DIR_PATH + "/0/" + FILE_NAME, instanceMetadata))
428+
.isInstanceOf(IllegalArgumentException.class);
429+
}
430+
414431
@Test
415432
public void testLocalPathNonExistingDirs()
416433
{

0 commit comments

Comments
 (0)