Skip to content

feat(url): percent-encode filename path segments in copied URLs#3

Open
erbanku wants to merge 2 commits into
superhumancorp:mainfrom
erbanku:main
Open

feat(url): percent-encode filename path segments in copied URLs#3
erbanku wants to merge 2 commits into
superhumancorp:mainfrom
erbanku:main

Conversation

@erbanku

@erbanku erbanku commented Jul 3, 2026

Copy link
Copy Markdown

Spaces and special characters in filenames were copied as-is into the clipboard URL instead of being percent-encoded, causing links to fail in some clients.

  • Add percent_encode_segment and percent_encode_r2_key helpers in the Rust engine and apply them in build_public_url, fixing the root source that writes to history DB and powers notification actions
  • Encode each r2Key path segment in QueueViewModel.copyURL and HistoryViewModel.resolveURL using urlPathAllowed, preserving / separators between segments
  • Re-encode legacy unencoded stored URLs in the history fallback path via URLComponents for backward compatibility
  • Add four new Rust unit tests covering spaces, parens, multi-segment paths, and unreserved character pass-through
  • Fix pre-existing compile error in config.rs test (missing allow_anonymous_telemetry field)

Closes #2

Spaces and special characters in filenames were copied as-is into the
clipboard URL instead of being percent-encoded, causing links to fail
in some clients.

- Add `percent_encode_segment` and `percent_encode_r2_key` helpers in
  the Rust engine and apply them in `build_public_url`, fixing the root
  source that writes to history DB and powers notification actions
- Encode each r2Key path segment in `QueueViewModel.copyURL` and
  `HistoryViewModel.resolveURL` using `urlPathAllowed`, preserving `/`
  separators between segments
- Re-encode legacy unencoded stored URLs in the history fallback path
  via `URLComponents` for backward compatibility
- Add four new Rust unit tests covering spaces, parens, multi-segment
  paths, and unreserved character pass-through
- Fix pre-existing compile error in config.rs test (missing
  `allow_anonymous_telemetry` field)

Closes superhumancorp#2
@erbanku
erbanku marked this pull request as ready for review July 3, 2026 06:59
Copilot AI review requested due to automatic review settings July 3, 2026 06:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes URL generation/copying so filenames containing spaces and other characters are percent-encoded per-path-segment (without encoding / separators), improving link reliability across clients while keeping legacy history entries working.

Changes:

  • Add RFC 3986 segment percent-encoding in the Rust engine’s build_public_url and expand unit test coverage.
  • Encode r2Key path segments when copying URLs from Queue and resolving URLs from History, including a legacy-history fallback re-encode.
  • Fix a Rust config test compile failure by adding the missing allow_anonymous_telemetry field.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
app/R2Drop/App/Queue/QueueViewModel.swift Percent-encodes each r2Key path segment when copying public URLs.
app/R2Drop/App/History/HistoryViewModel.swift Encodes r2Key segments for custom domains and re-encodes stored legacy URLs for backward compatibility.
app/engine/r2-core/src/runner.rs Adds strict per-segment percent-encoding in build_public_url and new unit tests.
app/engine/r2-core/src/config.rs Fixes test setup by including allow_anonymous_telemetry and minor formatting changes.
Comments suppressed due to low confidence (1)

app/R2Drop/App/Queue/QueueViewModel.swift:345

  • base.hasPrefix("http") treats any custom domain starting with "http" (e.g. httpfiles.example.com) as already including a scheme, producing a schemeless URL. Check for http:// or https:// explicitly before omitting the default scheme.
        if let domain = account.customDomain, !domain.isEmpty {
            let base = domain.hasSuffix("/") ? String(domain.dropLast()) : domain
            let scheme = base.hasPrefix("http") ? "" : "https://"
            url = "\(scheme)\(base)/\(r2Key)"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// Build URL from custom domain + r2Key.
// Percent-encode each segment without encoding '/' separators.
let base = domain.hasSuffix("/") ? String(domain.dropLast()) : domain
let scheme = base.hasPrefix("http") ? "" : "https://"
Comment on lines +93 to +97
if let components = URLComponents(string: entry.url),
let host = components.host
{
let scheme = components.scheme ?? "https"
let encodedPath = components.path
Comment thread app/engine/r2-core/src/config.rs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Copy URL does not percent-encode spaces and special characters in filenames

2 participants