Skip to content

Commit

Permalink
Only fallback for a number of grandfathered instances. (#23346)
Browse files Browse the repository at this point in the history
Fetching storage by storage document id is not allowed.

GitOrigin-RevId: 5212b04327500b266d62e30041c85c9f9a02ce14
  • Loading branch information
Preslav Le authored and Convex, Inc. committed Mar 12, 2024
1 parent e7f868f commit 4c9a2fc
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
6 changes: 6 additions & 0 deletions crates/common/src/knobs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,3 +871,9 @@ pub static MAX_BACKEND_PUBLIC_API_REQUEST_SIZE: LazyLock<usize> =
/// some maximum period of time after which they checkpoint unconditionally.
pub static DATABASE_WORKERS_MIN_COMMITS: LazyLock<usize> =
LazyLock::new(|| env_config("DATABASE_WORKERS_MIN_COMMITS", 100));

/// We have temporary grandfathered a few instances to be able to use storage by
/// using virtual document id until the developers can remove this incorrect
/// usage.
pub static ALLOW_STORAGE_GET_VIA_DOCUMENT_ID: LazyLock<bool> =
LazyLock::new(|| env_config("ALLOW_STORAGE_GET_VIA_DOCUMENT_ID", false));
32 changes: 19 additions & 13 deletions crates/local_backend/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use common::{
},
HttpResponseError,
},
knobs::ALLOW_STORAGE_GET_VIA_DOCUMENT_ID,
sha256::DigestHeader,
};
use errors::ErrorMetadata;
Expand All @@ -48,10 +49,7 @@ use file_storage::{
};
use futures::StreamExt;
use http::StatusCode;
use model::file_storage::{
types::StorageUuid,
FileStorageId,
};
use model::file_storage::FileStorageId;
use serde::{
Deserialize,
Serialize,
Expand Down Expand Up @@ -123,16 +121,24 @@ pub async fn storage_get(
// Query(QueryParams { token }): Query<QueryParams>,
range: Result<TypedHeader<Range>, TypedHeaderRejection>,
) -> Result<Response, HttpResponseError> {
let file_storage_id = match uuid.parse::<StorageUuid>() {
Ok(uuid) => FileStorageId::LegacyStorageId(uuid),
let storage_uuid = uuid.parse().context(ErrorMetadata::bad_request(
"InvalidStoragePath",
format!("Invalid storage path: \"{uuid}\" is not a valid UUID string."),
));
let file_storage_id = match storage_uuid {
Ok(storage_uuid) => FileStorageId::LegacyStorageId(storage_uuid),
Err(err) => {
// TODO: Delete this fallback.
// Fallback to parsing as uuid or virtual document_id. We should not
// allow the latter but there might be users that rely on it. For now,
// log if the fallback succeeds and allow it.
let file_storage_id = uuid.parse()?;
report_error(&mut err.context("Improper storage_get() use"));
file_storage_id
if *ALLOW_STORAGE_GET_VIA_DOCUMENT_ID {
// TODO: Delete this fallback.
// Fallback to parsing as uuid or virtual document_id. We should not
// allow the latter but there might be users that rely on it. For now,
// log if the fallback succeeds and allow it.
let file_storage_id = uuid.parse()?;
report_error(&mut err.context("Improper storage_get() use"));
file_storage_id
} else {
return Err(err.into());
}
},
};
// TODO(CX-3065) figure out deterministic repeatable tokens
Expand Down

0 comments on commit 4c9a2fc

Please sign in to comment.