diff --git a/fileglancer/alembic/versions/20b763c28c4f_add_url_prefix_to_proxied_paths.py b/fileglancer/alembic/versions/20b763c28c4f_add_url_prefix_to_proxied_paths.py new file mode 100644 index 000000000..0a507d6e7 --- /dev/null +++ b/fileglancer/alembic/versions/20b763c28c4f_add_url_prefix_to_proxied_paths.py @@ -0,0 +1,35 @@ +"""add url_prefix to proxied_paths + +Revision ID: 20b763c28c4f +Revises: a3d7cc6e95e8 +Create Date: 2026-04-09 16:11:10.155619 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '20b763c28c4f' +down_revision = 'a3d7cc6e95e8' +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column('proxied_paths', sa.Column('url_prefix', sa.String(), server_default='', nullable=False)) + # Backfill: set url_prefix to basename of path for existing rows + proxied_paths = sa.table('proxied_paths', sa.column('path', sa.String), sa.column('url_prefix', sa.String)) + conn = op.get_bind() + rows = conn.execute(sa.select(proxied_paths.c.path).distinct()).fetchall() + for (path,) in rows: + basename = path.rsplit('/', 1)[-1] if '/' in path else path + conn.execute( + proxied_paths.update() + .where(proxied_paths.c.path == path) + .values(url_prefix=basename) + ) + + +def downgrade() -> None: + op.drop_column('proxied_paths', 'url_prefix') \ No newline at end of file diff --git a/fileglancer/database.py b/fileglancer/database.py index f1f885923..0cdf458d8 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -95,6 +95,7 @@ class ProxiedPathDB(Base): sharing_name = Column(String, nullable=False) fsp_name = Column(String, nullable=False) path = Column(String, nullable=False) + url_prefix = Column(String, nullable=False, server_default="") created_at = Column(DateTime, nullable=False, default=lambda: datetime.now(UTC)) updated_at = Column(DateTime, nullable=False, default=lambda: datetime.now(UTC), onupdate=lambda: datetime.now(UTC)) @@ -589,7 +590,7 @@ def _validate_proxied_path(session: Session, fsp_name: str, path: str) -> None: raise ValueError(f"Path {path} is not accessible relative to {fsp_name}") -def create_proxied_path(session: Session, username: str, sharing_name: str, fsp_name: str, path: str) -> ProxiedPathDB: +def create_proxied_path(session: Session, username: str, sharing_name: str, fsp_name: str, path: str, url_prefix: str = "") -> ProxiedPathDB: """Create a new proxied path""" _validate_proxied_path(session, fsp_name, path) @@ -601,6 +602,7 @@ def create_proxied_path(session: Session, username: str, sharing_name: str, fsp_ sharing_name=sharing_name, fsp_name=fsp_name, path=path, + url_prefix=url_prefix, created_at=now, updated_at=now ) @@ -614,6 +616,7 @@ def create_proxied_path(session: Session, username: str, sharing_name: str, fsp_ return proxied_path + def update_proxied_path(session: Session, username: str, sharing_key: str, @@ -630,6 +633,7 @@ def update_proxied_path(session: Session, if new_sharing_name: proxied_path.sharing_name = new_sharing_name + proxied_path.url_prefix = new_sharing_name if new_fsp_name: proxied_path.fsp_name = new_fsp_name diff --git a/fileglancer/model.py b/fileglancer/model.py index 2ff0e0c47..553ab178c 100644 --- a/fileglancer/model.py +++ b/fileglancer/model.py @@ -135,7 +135,7 @@ class ProxiedPath(BaseModel): description="The sharing key is part of the URL proxy path. It is used to uniquely identify the proxied path." ) sharing_name: str = Field( - description="The sharing path is part of the URL proxy path. It is mainly used to provide file extension information to the client." + description="A display-only label for the data link. Does not appear in the URL." ) path: str = Field( description="The path relative to the file share path mount point" @@ -149,6 +149,9 @@ class ProxiedPath(BaseModel): updated_at: datetime = Field( description="When this proxied path was last updated" ) + url_prefix: str = Field( + description="The URL path prefix that appears after the sharing key in the proxy URL" + ) url: Optional[HttpUrl] = Field( description="The URL for accessing the data via the proxy", default=None diff --git a/fileglancer/server.py b/fileglancer/server.py index de84bd50e..899cc423c 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -1,5 +1,6 @@ import logging import os +import re import sys import pwd import grp @@ -95,7 +96,7 @@ def _convert_external_bucket(db_bucket: db.ExternalBucketDB) -> ExternalBucket: def _convert_proxied_path(db_path: db.ProxiedPathDB, external_proxy_url: Optional[HttpUrl]) -> ProxiedPath: """Convert a database ProxiedPathDB model to a Pydantic ProxiedPath model""" if external_proxy_url: - url = f"{external_proxy_url}/{db_path.sharing_key}/{quote(db_path.sharing_name)}" + url = f"{external_proxy_url}/{db_path.sharing_key}/{quote(db_path.url_prefix, safe='/')}" else: logger.warning(f"No external proxy URL was provided, proxy links will not be available.") url = None @@ -105,12 +106,33 @@ def _convert_proxied_path(db_path: db.ProxiedPathDB, external_proxy_url: Optiona sharing_name=db_path.sharing_name, fsp_name=db_path.fsp_name, path=db_path.path, + url_prefix=db_path.url_prefix, created_at=db_path.created_at, updated_at=db_path.updated_at, url=url ) +# Regex: allow unreserved URI chars (RFC 3986), plus / for path separators and common safe chars +_VALID_URL_PREFIX_RE = re.compile(r'^[A-Za-z0-9\-._~/!@$&\'()*+,;:=%]+$') + + +def _validate_url_prefix(url_prefix: str) -> None: + """Validate that a url_prefix is non-empty and contains only URL-safe characters.""" + if not url_prefix or not url_prefix.strip(): + raise HTTPException(status_code=400, detail="Data link name must not be empty") + if not _VALID_URL_PREFIX_RE.match(url_prefix): + invalid_chars = set(c for c in url_prefix if not re.match(r"[A-Za-z0-9\-._~/!@$&'()*+,;:=]", c)) + raise HTTPException( + status_code=400, + detail=f"Data link name contains invalid URL characters: {' '.join(sorted(invalid_chars))}" + ) + if url_prefix.startswith('/') or url_prefix.endswith('/'): + raise HTTPException(status_code=400, detail="Data link name must not start or end with /") + if '//' in url_prefix: + raise HTTPException(status_code=400, detail="Data link name must not contain consecutive slashes") + + def _convert_ticket(db_ticket: db.TicketDB) -> Ticket: return Ticket( username=db_ticket.username, @@ -199,29 +221,43 @@ def _get_user_context(username: str) -> UserContext: return CurrentUserContext() - def _get_file_proxy_client(sharing_key: str, sharing_name: str) -> Tuple[FileProxyClient, UserContext] | Tuple[Response, None]: + def _get_file_proxy_client(sharing_key: str, captured_path: str) -> Tuple[FileProxyClient | Response, UserContext | None, str]: + """Resolve a sharing key and captured path to a FileProxyClient. + + Returns (client, user_context, subpath) on success, or (error_response, None, "") on failure. + """ + def try_strip_prefix(captured: str, prefix: str) -> str | None: + if captured == prefix: + return "" + if captured.startswith(prefix + "/"): + return captured[len(prefix) + 1:] + return None + with db.get_db_session(settings.db_url) as session: proxied_path = db.get_proxied_path_by_sharing_key(session, sharing_key) if not proxied_path: - return get_nosuchbucket_response(sharing_name), None - - # Vol-E viewer sends URLs with literal % characters (not URL-encoded) - # FastAPI automatically decodes path parameters - % chars are treated as escapes, creating a garbled sharing_name if they're present - # We therefore need to handle two cases: - # 1. Properly encoded requests (sharing_name matches DB value of proxied_path.sharing_name) - # 2. Vol-E's unencoded requests (unquote(proxied_path.sharing_name) matches the garbled request value) - if proxied_path.sharing_name != sharing_name and unquote(proxied_path.sharing_name) != sharing_name: - return get_error_response(404, "NoSuchKey", f"Sharing name mismatch for sharing key {sharing_key}", sharing_name), None + return get_nosuchbucket_response(captured_path), None, "" + + # Match captured_path against the stored url_prefix. + # The unquote() fallback handles clients like Vol-E viewer that send URLs + # with literal % characters instead of proper URL encoding — FastAPI + # auto-decodes path params, so we need to match the decoded form too. + subpath = try_strip_prefix(captured_path, proxied_path.url_prefix) + if subpath is None: + subpath = try_strip_prefix(captured_path, unquote(proxied_path.url_prefix)) + if subpath is None: + return get_error_response(404, "NoSuchKey", f"Path mismatch for sharing key {sharing_key}", captured_path), None, "" fsp = db.get_file_share_path(session, proxied_path.fsp_name) if not fsp: - return get_error_response(400, "InvalidArgument", f"File share path {proxied_path.fsp_name} not found", sharing_name), None + return get_error_response(400, "InvalidArgument", f"File share path {proxied_path.fsp_name} not found", captured_path), None, "" # Expand ~ to user's home directory before constructing the mount path expanded_mount_path = os.path.expanduser(fsp.mount_path) mount_path = f"{expanded_mount_path}/{proxied_path.path}" + target_name = captured_path.rsplit('/', 1)[-1] if captured_path else os.path.basename(proxied_path.path) # Use 256KB buffer for better performance on network filesystems - return FileProxyClient(proxy_kwargs={'target_name': sharing_name}, path=mount_path, buffer_size=256*1024), _get_user_context(proxied_path.username) + return FileProxyClient(proxy_kwargs={'target_name': target_name}, path=mount_path, buffer_size=256*1024), _get_user_context(proxied_path.username), subpath @asynccontextmanager @@ -849,14 +885,20 @@ async def delete_neuroglancer_short_link(short_key: str = Path(..., description= description="Create a new proxied path") async def create_proxied_path(fsp_name: str = Query(..., description="The name of the file share path that this proxied path is associated with"), path: str = Query(..., description="The path relative to the file share path mount point"), + url_prefix: Optional[str] = Query(None, description="The URL path prefix after the sharing key. Defaults to basename of path."), username: str = Depends(get_current_user)): - sharing_name = os.path.basename(path) - logger.info(f"Creating proxied path for {username} with sharing name {sharing_name} and fsp_name {fsp_name} and path {path}") + if url_prefix is None: + url_prefix = quote(os.path.basename(path), safe='/') + elif not _VALID_URL_PREFIX_RE.match(url_prefix): + url_prefix = quote(url_prefix, safe='/') + _validate_url_prefix(url_prefix) + sharing_name = url_prefix + logger.info(f"Creating proxied path for {username} with sharing name {sharing_name} and fsp_name {fsp_name} and path {path} (url_prefix={url_prefix})") with db.get_db_session(settings.db_url) as session: with _get_user_context(username): # Necessary to validate the user can access the proxied path try: - new_path = db.create_proxied_path(session, username, sharing_name, fsp_name, path) + new_path = db.create_proxied_path(session, username, sharing_name, fsp_name, path, url_prefix=url_prefix) return _convert_proxied_path(new_path, settings.external_proxy_url) except ValueError as e: logger.error(f"Error creating proxied path: {e}") @@ -970,12 +1012,10 @@ async def get_neuroglancer_short_links(request: Request, return NeuroglancerShortLinkResponse(links=links) - @app.get("/files/{sharing_key}/{sharing_name}") - @app.get("/files/{sharing_key}/{sharing_name}/{path:path}") + @app.get("/files/{sharing_key}/{path:path}") async def target_dispatcher(request: Request, sharing_key: str, - sharing_name: str, - path: str | None = '', + path: str = '', list_type: Optional[int] = Query(None, alias="list-type"), continuation_token: Optional[str] = Query(None, alias="continuation-token"), delimiter: Optional[str] = Query(None, alias="delimiter"), @@ -988,7 +1028,7 @@ async def target_dispatcher(request: Request, if 'acl' in request.query_params: return get_read_access_acl() - client, ctx = _get_file_proxy_client(sharing_key, sharing_name) + client, ctx, subpath = _get_file_proxy_client(sharing_key, path) if isinstance(client, Response): return client @@ -1005,7 +1045,7 @@ async def target_dispatcher(request: Request, # Open file in user context, then immediately exit # The file descriptor retains access rights after we switch back to root with ctx: - handle = await client.open_object(path, range_header) + handle = await client.open_object(subpath, range_header) # Context exited! Now stream without holding the lock if isinstance(handle, ObjectHandle): @@ -1015,14 +1055,14 @@ async def target_dispatcher(request: Request, return handle - @app.head("/files/{sharing_key}/{sharing_name}/{path:path}") - async def head_object(sharing_key: str, sharing_name: str, path: str): + @app.head("/files/{sharing_key}/{path:path}") + async def head_object(sharing_key: str, path: str = ''): try: - client, ctx = _get_file_proxy_client(sharing_key, sharing_name) + client, ctx, subpath = _get_file_proxy_client(sharing_key, path) if isinstance(client, Response): return client with ctx: - return await client.head_object(path) + return await client.head_object(subpath) except: logger.opt(exception=sys.exc_info()).info("Error requesting head") return get_error_response(500, "InternalError", "Error requesting HEAD", path) diff --git a/frontend/src/__tests__/mocks/handlers.ts b/frontend/src/__tests__/mocks/handlers.ts index e61b2d9b3..609be7049 100644 --- a/frontend/src/__tests__/mocks/handlers.ts +++ b/frontend/src/__tests__/mocks/handlers.ts @@ -18,16 +18,36 @@ export const handlers = [ return HttpResponse.json({ paths: [] }, { status: 200 }); }), - http.post('/api/proxied-path', () => { + http.post('/api/proxied-path', ({ request }) => { + const url = new URL(request.url); + const path = url.searchParams.get('path') || '/test/path'; + const pathBasename = path.split('/').pop() || path; + const urlPrefix = url.searchParams.get('url_prefix') || pathBasename; return HttpResponse.json({ username: 'testuser', sharing_key: 'testkey', - sharing_name: 'testshare', + sharing_name: pathBasename, + path: path, + fsp_name: url.searchParams.get('fsp_name') || 'test_fsp', + created_at: '2025-07-08T15:56:42.588942', + updated_at: '2025-07-08T15:56:42.588942', + url: 'http://127.0.0.1:7878/files/testkey/' + urlPrefix, + url_prefix: urlPrefix + }); + }), + + http.put('/api/proxied-path/:sharingKey', async ({ params, request }) => { + const { sharingKey } = params; + const body = (await request.json()) as { sharing_name?: string }; + return HttpResponse.json({ + username: 'testuser', + sharing_key: sharingKey, + sharing_name: body.sharing_name || 'testshare', path: '/test/path', fsp_name: 'test_fsp', created_at: '2025-07-08T15:56:42.588942', updated_at: '2025-07-08T15:56:42.588942', - url: 'http://127.0.0.1:7878/files/testkey/test/path' + url: 'http://127.0.0.1:7878/files/' + sharingKey + '/path' }); }), diff --git a/frontend/src/components/ui/Dialogs/DataLink.tsx b/frontend/src/components/ui/Dialogs/DataLink.tsx index d11bbe27d..a09f17110 100644 --- a/frontend/src/components/ui/Dialogs/DataLink.tsx +++ b/frontend/src/components/ui/Dialogs/DataLink.tsx @@ -1,20 +1,27 @@ /* eslint-disable react/destructuring-assignment */ // Props are used for TypeScript type narrowing purposes and cannot be destructured at the beginning -import { useState, SetStateAction } from 'react'; +import { useState, useMemo, SetStateAction } from 'react'; import type { ReactNode, Dispatch } from 'react'; import { Button, Typography } from '@material-tailwind/react'; +import { Link } from 'react-router'; import type { ProxiedPath } from '@/contexts/ProxiedPathContext'; import { useFileBrowserContext } from '@/contexts/FileBrowserContext'; +import { validateUrlPrefix } from '@/hooks/useDataToolLinks'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { useZoneAndFspMapContext } from '@/contexts/ZonesAndFspMapContext'; -import { getPreferredPathForDisplay, makeMapKey } from '@/utils'; +import { makeMapKey } from '@/utils'; +import { + getPreferredPathForDisplay, + joinPaths, + normalizePosixStylePath +} from '@/utils/pathHandling'; import type { FileSharePath } from '@/shared.types'; import type { PendingToolKey } from '@/hooks/useZarrMetadata'; import FgDialog from './FgDialog'; import TextWithFilePath from './TextWithFilePath'; -import AutomaticLinksToggle from '@/components/ui/PreferencesPage/DataLinkOptions'; +import DataLinkOptions from '@/components/ui/PreferencesPage/DataLinkOptions'; import DeleteBtn from '@/components/ui/buttons/DeleteBtn'; interface CommonDataLinkDialogProps { @@ -25,7 +32,7 @@ interface CommonDataLinkDialogProps { interface CreateLinkFromToolsProps extends CommonDataLinkDialogProps { tools: true; action: 'create'; - onConfirm: () => Promise; + onConfirm: (urlPrefixOverride?: string) => Promise; onCancel: () => void; setPendingToolKey: Dispatch>; } @@ -33,7 +40,7 @@ interface CreateLinkFromToolsProps extends CommonDataLinkDialogProps { interface CreateLinkNotFromToolsProps extends CommonDataLinkDialogProps { tools: false; action: 'create'; - onConfirm: () => Promise; + onConfirm: (urlPrefixOverride?: string) => Promise; onCancel: () => void; } @@ -50,14 +57,17 @@ type DataLinkDialogProps = | DeleteLinkDialogProps; function CreateLinkBtn({ - onConfirm + onConfirm, + disabled }: { readonly onConfirm: () => Promise; + readonly disabled?: boolean; }) { return (