UFAL/Fix: generate correct curl download URLs using backend handle endpoint#1215
UFAL/Fix: generate correct curl download URLs using backend handle endpoint#1215milanmajchrak wants to merge 5 commits intodtq-devfrom
Conversation
Updates the curl command generation to use the new backend endpoint
GET /api/core/bitstreams/handle/{prefix}/{suffix}/{filename} instead
of the non-existent /api/bitstream/{handle}/{seq}/{filename}.
Key changes:
- Uses correct backend endpoint path: /core/bitstreams/handle/{handle}/
- Removes unnecessary sequence index from URLs (uses filename only)
- Quotes the URL to prevent shell brace expansion
- For single file, uses -o with explicit filename
Fixes: #1210
…rl -O curl -O uses the URL path as the saved filename, so percent-encoded characters (e.g. %20, %2B, %28) stay encoded in the output file. Now generates separate 'curl -o realname url' for each file joined with &&, ensuring files are saved with their actual names.
There was a problem hiding this comment.
Pull request overview
This pull request fixes the curl command generation for downloading bitstreams from DSpace items. The previous implementation referenced a non-existent backend endpoint /api/bitstream/{handle}/{seq}/{filename}, which has been replaced with the correct endpoint /api/core/bitstreams/handle/{prefix}/{suffix}/{filename}. The fix enables users to download files via command line using proper curl commands.
Changes:
- Updated endpoint path from
/bitstream/to/core/bitstreams/handle/ - Removed sequence index from URL construction (now uses filename only)
- Added URL quoting to prevent shell brace expansion
- Added
-Jflag to curl command to use Content-Disposition header filename - Introduced
encodeFilenameForUrl()method for proper filename URL encoding
src/app/item-page/clarin-files-section/clarin-files-section.component.ts
Show resolved
Hide resolved
| private encodeFilenameForUrl(filename: string): string { | ||
| return encodeURIComponent(filename).replace(/[()]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase()); | ||
| } |
There was a problem hiding this comment.
The encodeFilenameForUrl method duplicates logic from the existing encodeRFC3986URIComponent utility function in clarin-shared-util.ts. The key difference is that the existing utility decodes first with decodeURIComponent before encoding, which properly handles filenames that may already be partially encoded. Consider reusing the existing utility to avoid code duplication and potential encoding inconsistencies.
src/app/item-page/clarin-files-section/clarin-files-section.component.ts
Outdated
Show resolved
Hide resolved
| generateCurlCommand() { | ||
| const fileNames = this.listOfFiles.value.map((file: MetadataBitstream) => { | ||
| if (file.canPreview) { | ||
| this.canShowCurlDownload = true; | ||
| } | ||
|
|
||
| return file.name; | ||
| }); | ||
|
|
||
| // Generate curl command for individual bitstream downloads | ||
| const baseUrl = `${this.halService.getRootHref()}/bitstream/${this.itemHandle}`; | ||
| const fileNamesFormatted = fileNames.map((fileName, index) => `/${index}/${fileName}`).join(','); | ||
| this.command = `curl -O ${baseUrl}{${fileNamesFormatted}}`; | ||
| // Generate curl command for individual bitstream downloads by handle + filename. | ||
| // Uses the backend endpoint: /api/core/bitstreams/handle/{prefix}/{suffix}/{filename} | ||
| // -J tells curl to use the filename from the Content-Disposition header (the real name) | ||
| // instead of the percent-encoded URL path. | ||
| const baseUrl = `${this.halService.getRootHref()}/core/bitstreams/handle/${this.itemHandle}`; | ||
| const fileNamesFormatted = fileNames.map(fileName => `/${this.encodeFilenameForUrl(fileName)}`).join(','); | ||
| this.command = `curl -OJ "${baseUrl}{${fileNamesFormatted}}"`; | ||
|
|
||
| } | ||
|
|
||
| /** | ||
| * Encode a filename for use in a URL path segment. | ||
| * Encodes special characters (spaces, parentheses, +, etc.) using percent-encoding. | ||
| */ | ||
| private encodeFilenameForUrl(filename: string): string { | ||
| return encodeURIComponent(filename).replace(/[()]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase()); | ||
| } |
There was a problem hiding this comment.
The new generateCurlCommand method and encodeFilenameForUrl method lack test coverage. Given that this codebase has comprehensive automated testing with over 4800 unit tests, consider adding tests for the curl command generation logic to cover various scenarios such as single file, multiple files, files with special characters (spaces, parentheses, brackets), and files with various encodings.
Problem description
Updates the curl command generation to use the new backend endpoint GET /api/core/bitstreams/handle/{prefix}/{suffix}/{filename} instead of the non-existent /api/bitstream/{handle}/{seq}/{filename}.
Key changes:
Fixes: #1210
Sync verification
If en.json5 or cs.json5 translation files were updated:
yarn run sync-i18n -t src/assets/i18n/cs.json5 -ito synchronize messages, and changes are included in this PR.Manual Testing (if applicable)
Copilot review