diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala index 4da407fd31c..ac9b5a9d62e 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala @@ -423,6 +423,8 @@ class DatasetResource extends LazyLogging { throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE) } + ResourceNaming.validateVersionDescription(versionName) + val dataset = getDatasetByID(ctx, did) val datasetName = dataset.getName val repositoryName = dataset.getRepositoryName diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala index 80a69cbda2a..a9a6fa6f1d0 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala @@ -741,6 +741,8 @@ class ModelResource extends LazyLogging { throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) } + ResourceNaming.validateVersionDescription(versionName) + val model = getModelByID(ctx, mid) val modelName = model.getName val repositoryName = model.getRepositoryName diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/ResourceNaming.scala b/file-service/src/main/scala/org/apache/texera/service/resource/ResourceNaming.scala index 7a71362256c..b378e74d12f 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/ResourceNaming.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/ResourceNaming.scala @@ -54,6 +54,22 @@ object ResourceNaming { } } + /** + * Rejects a version description containing "/". The version name is a segment of the logical + * path //ownerEmail/resourceName/versionName/file, which FileResolver splits on "/", so + * none of such a version's files could be opened. A null or empty description is allowed: the + * version is then named by its number alone. + * + * @throws jakarta.ws.rs.BadRequestException if the description contains a "/". + */ + def validateVersionDescription(description: String): Unit = + if (description != null && description.contains("/")) { + throw new BadRequestException( + "Invalid version description: '/' is not allowed because the version name is part of " + + "the paths of its files." + ) + } + /** * Validates a file path supplied for a resource's contents. * diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala index db4c2437ebd..9361e46a70f 100644 --- a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala @@ -3975,4 +3975,21 @@ class DatasetResourceSpec assertStatus(ex, 400) datasetResource.getDatasetVersionList(dataset.getDid, sessionUser) should have size 1 } + + it should "reject a description containing a slash without committing" in { + val dataset = seedDatasetWithStagedFile("version-slash") + + val ex = intercept[WebApplicationException] { + createVersion(dataset, "2024/01 snapshot") + } + assertStatus(ex, 400) + + // Nothing was committed: the file is still staged and no version row exists, so the retry + // with a usable description succeeds as the first version. + LakeFSStorageClient + .retrieveUncommittedObjects(dataset.getRepositoryName) + .map(_.getPath) shouldBe List("staged.txt") + val retried = createVersion(dataset, "2024-01 snapshot") + retried.datasetVersion.getName shouldBe "v1 - 2024-01 snapshot" + } } diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/ModelUploadResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/ModelUploadResourceSpec.scala index f4f300b7dcf..70bfc0d0d3a 100644 --- a/file-service/src/test/scala/org/apache/texera/service/resource/ModelUploadResourceSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/resource/ModelUploadResourceSpec.scala @@ -326,6 +326,21 @@ class ModelUploadResourceSpec .map(_.getName) should contain("weights.pt") } + it should "reject a description containing a slash without committing to LakeFS" in { + val model = newModel() + val mid = model.model.getMid + + uploadOneShot(mid, "weights.pt", Array.fill[Byte](32)(0x1)).getStatus shouldEqual 200 + + intercept[BadRequestException] { + modelResource.createModelVersion("2024/01 snapshot", mid, sessionUser) + } + + // The staged change survived, so a retry with a usable description works. + val recovered = modelResource.createModelVersion("2024-01 snapshot", mid, sessionUser) + recovered.modelVersion.getName should endWith("2024-01 snapshot") + } + "retrieveModelVersionRootFileNodes" should "404 for a version belonging to another model" in { val modelA = newModel() val modelB = newModel() diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/ResourceNamingSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/ResourceNamingSpec.scala index 71e6812eb6b..fd23cdd2438 100644 --- a/file-service/src/test/scala/org/apache/texera/service/resource/ResourceNamingSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/resource/ResourceNamingSpec.scala @@ -23,9 +23,9 @@ import jakarta.ws.rs.BadRequestException import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -// Focused unit tests for ResourceNaming.validateAndNormalizeFilePathOrThrow, which guards -// every upload/lookup path. These call the pure helper directly and avoid the heavy -// DatasetResourceSpec integration harness (DB + LakeFS). +// Focused unit tests for ResourceNaming's pure validators: validateAndNormalizeFilePathOrThrow, +// which guards every upload/lookup path, and validateVersionDescription. These call the helpers +// directly and avoid the heavy DatasetResourceSpec integration harness (DB + LakeFS). class ResourceNamingSpec extends AnyFlatSpec with Matchers { "validateAndNormalizeFilePathOrThrow" should "reject a null path" in { @@ -76,4 +76,25 @@ class ResourceNamingSpec extends AnyFlatSpec with Matchers { "a/./b/../c.csv" ) shouldBe "a/c.csv" } + + "validateVersionDescription" should "reject a description containing a slash anywhere" in { + Seq("/", "2024/01 snapshot", "/leading", "trailing/", "a//b").foreach { description => + val ex = intercept[BadRequestException] { + ResourceNaming.validateVersionDescription(description) + } + ex.getMessage should include("'/'") + } + } + + it should "accept a null or empty description, which names the version by its number alone" in { + ResourceNaming.validateVersionDescription(null) + ResourceNaming.validateVersionDescription("") + } + + it should "accept any description without a slash" in { + Seq("second upload", "2024-01 snapshot", "a\\b", "naïve 数据 \uD83D\uDE00", " ") + .foreach { + ResourceNaming.validateVersionDescription + } + } } diff --git a/frontend/src/app/dashboard/component/user/version-uploader/version-uploader.component.html b/frontend/src/app/dashboard/component/user/version-uploader/version-uploader.component.html index 384d706f600..1829cc611e9 100644 --- a/frontend/src/app/dashboard/component/user/version-uploader/version-uploader.component.html +++ b/frontend/src/app/dashboard/component/user/version-uploader/version-uploader.component.html @@ -158,14 +158,24 @@ [(ngModel)]="versionName" placeholder="Describe the new version (Optional)" [disabled]="isCreatingVersion" + [attr.aria-invalid]="versionNameHasSlash ? 'true' : null" + [attr.aria-describedby]="versionNameHasSlash ? 'version-name-error' : null" (keydown.enter)="onClickCreateVersion()" class="version-input" /> +