Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix redirect URLs of bitstreams with spaces in filename #3876

Conversation

msarmie
Copy link
Contributor

@msarmie msarmie commented Jan 21, 2025

References

Description

Removes encodeURIComponent in bitstream-data.service.ts#L174 because RequestParam is already encoding values

This extra encoding causes spaces to be encoded twice %20 to %2520 and URL redirect fails with 404 error.

@tdonohue tdonohue added bug component: SEO Search Engine Optimization 1 APPROVAL pull request only requires a single approval to merge port to main This PR needs to be ported to `main` branch for the next major release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release and removed port to main This PR needs to be ported to `main` branch for the next major release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Jan 21, 2025
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Good catch, @msarmie ! I tested this and it fixes the bug you've described.

Somewhat strangely, this bug only appears in 7.x. The line of code in 8.x or pre-9.0 does not include the encodeURIComponent() call.

So, this is definitely a bug specific to 7.x. I'm going to port your new unit test to 8.x and main to ensure that test exists on all branches (to avoid us breaking this behavior again)

@tdonohue tdonohue added this to the 7.6.3 milestone Jan 22, 2025
@tdonohue tdonohue merged commit 00ff0cc into DSpace:dspace-7_x Jan 22, 2025
15 checks passed
@dspace-bot
Copy link
Contributor

Backport failed for main, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin main
git worktree add -d .worktree/backport-3876-to-main origin/main
cd .worktree/backport-3876-to-main
git switch --create backport-3876-to-main
git cherry-pick -x 74d6dbb454cf40d5c418cc0c6cde05ea04de2103 2bd53d822abe8b811bcde9d613f68001a35ef3bf

@dspace-bot
Copy link
Contributor

Backport failed for dspace-8_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-8_x
git worktree add -d .worktree/backport-3876-to-dspace-8_x origin/dspace-8_x
cd .worktree/backport-3876-to-dspace-8_x
git switch --create backport-3876-to-dspace-8_x
git cherry-pick -x 74d6dbb454cf40d5c418cc0c6cde05ea04de2103 2bd53d822abe8b811bcde9d613f68001a35ef3bf

@tdonohue tdonohue removed port to main This PR needs to be ported to `main` branch for the next major release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Jan 23, 2025
@msarmie msarmie deleted the fix-bitsream-filename-encoding-url-dspace-7_x branch January 23, 2025 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge bug component: SEO Search Engine Optimization
Projects
Development

Successfully merging this pull request may close these issues.

3 participants