UFAL/fix: S3 presigned-URL Content-Disposition — spaces became '+', raw non-ASCII in filename param

[milanmajchrak]

Two defects in the S3 direct-download path, both spotted while doing #1368.

1. Spaces became +

String encoded = URLEncoder.encode(desiredFilename, StandardCharsets.UTF_8);

URLEncoder is form encoding, which differs from RFC 5987 on exactly one character: it emits + for a space where RFC 5987 wants %20. A client decoding filename* strictly gets my+file.txt. The other fork call sites already had the .replace("+", "%20") fixup; this one never did.

2. Raw non-ASCII left in the filename param

String safeName = desiredFilename.replaceAll("[\r\n\"]", "_");

Only CR, LF and quotes were replaced, so Příliš.pdf went into the filename param as raw UTF-8 bytes. That param is ISO-8859-1 only — the same bug class #1368 fixes elsewhere.

Both are fixed by copying vanilla's createFallbackAsciiName / createEncodedUtf8Name into this class, the way the other fork-only endpoints do. S3DirectDownloadServiceImpl has no upstream counterpart, so copying costs nothing and keeps its behaviour tracking vanilla. No shared fork helper is introduced.

Deviations from vanilla — both pre-existing behaviour of this class, both marked in code

• CR/LF are dropped — this class already did that to prevent header injection. Vanilla's fallback does not strip them ([^\x00-\x7F] only removes characters outside 00–7F, and CR/LF are inside), so copying it verbatim would have removed an existing protection.
• \ and " are escaped — vanilla omits this, and a quoted name breaks the header.

Other control characters, tab included, still pass through exactly as before this PR, so weirdFilename keeps its existing assertions unchanged.

Verified

• mvn test-compile on dspace-api — pass
• mvn checkstyle:check on dspace-api — 0 violations
• ⚠️ S3DirectDownloadServiceTest could not be run locally. It extends AbstractUnitTest, which boots the DSpace kernel, and the kernel fails to start in my checkout (No qualifying bean of type 'org.dspace.core.ProvenanceService') because target/testing is unpacked from a stale testEnvironment.zip in the local ~/.m2, and rebuilding that artifact needs a full root build. The identical failure reproduces on unmodified origin/dtq-dev, so it is environmental rather than caused by this change — but it does mean my edits to that test file are unverified and need CI to confirm them, in particular the new spacesAndDiacriticsInFilename assertion.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f220c1d0-41d7-4dd0-bb90-cf09c9ffd9b3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}