Copilot commented on code in PR #8446: URL: https://github.com/apache/texera/pull/8446#discussion_r3998541645
########## frontend/src/app/common/util/download-integrity.util.ts: ########## @@ -0,0 +1,43 @@ +/** + * 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. + */ + +import { HttpResponse } from "@angular/common/http"; +import { OperatorFunction } from "rxjs"; +import { map } from "rxjs/operators"; + +export class TruncatedDownloadError extends Error { + constructor( + readonly expectedBytes: number, + readonly receivedBytes: number + ) { + super(`Download truncated: expected ${expectedBytes} bytes but received ${receivedBytes}.`); + this.name = "TruncatedDownloadError"; + } +} + +export function verifyCompleteDownload(): OperatorFunction<HttpResponse<Blob>, Blob> { + return map(response => { + const blob = response.body ?? new Blob([]); + const declaredLength = Number(response.headers.get("Content-Length")); + if (Number.isFinite(declaredLength) && declaredLength > blob.size) { Review Comment: The version-ZIP callers added here never receive a length to check: both endpoints delegate to `ResourceUploadService.versionZipResponse`, which returns a `StreamingOutput` without `Content-Length` (`ResourceUploadService.scala:380-402`). This converts the missing header to zero and accepts every ZIP, including one truncated after the gateway, so the claimed defense-in-depth does not currently cover version ZIPs. Please provide an independent expected size/integrity signal for these streams, or remove the ineffective ZIP verification and scope the claim to presigned object downloads. ########## frontend/src/app/dashboard/service/user/model/model.service.ts: ########## @@ -183,9 +187,10 @@ export class ModelService { const endpointSegment = isLogin ? "presign-download" : "public-presign-download"; const endpoint = `${AppSettings.getApiEndpoint()}/${MODEL_BASE_URL}/${endpointSegment}?filePath=${encodeURIComponent(filePath)}`; - return this.http - .get<{ presignedUrl: string }>(endpoint) - .pipe(switchMap(({ presignedUrl }) => this.http.get(presignedUrl, { responseType: "blob" }))); + return this.http.get<{ presignedUrl: string }>(endpoint).pipe( + switchMap(({ presignedUrl }) => this.http.get(presignedUrl, { responseType: "blob", observe: "response" })), + verifyCompleteDownload() Review Comment: Both modified model download paths are still tested only with successful blobs in `model.service.spec.ts:181-215`; the new truncation behavior is integration-tested only for `DatasetService`. Add model-service regression cases for short presigned-file and version-ZIP responses that assert `TruncatedDownloadError`, so the model wiring introduced here is covered. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
