dspace-angular
dspace-angular copied to clipboard
Encode all `RequestParam` values
References
- Fixes #2724
- Fixes #2727
Description
This PR encodes all the RequestParam
s values. I added an optional parameter to still make it possible to provide already encoded values if needed. This has also an added advantage that it will throw errors for all values that are not created by using the new RequestParam
constructor, since the third parameter will be missing.
Instructions for Reviewers
List of changes in this PR:
- Encode the value received in
RequestParam
by default - Removed the
encodeURIComponent()
of values that were given as parameters toRequestParam
to prevent values from being encoded twice
See #2724 & #2727 for how to test this PR (#2727 is only reproducible in production mode)
Checklist
- [x] My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
- [x] My PR passes ESLint validation using
yarn lint
- [x] My PR doesn't introduce circular dependencies (verified via
yarn check-circ-deps
) - [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
- [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
- [x] If my PR includes new libraries/dependencies (in
package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation. - [x] If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
- [x] If my PR fixes an issue ticket, I've linked them together.
@tdonohue: Like discussed in #2727 I refactored this PR to encode all RequestParam
s, since this fixes both issues at the same time
@alexandrevryghem : Any chance of still moving this forward for 8.0? It appears the feedback in my review wasn't addressed yet. If this is turning out to be too difficult, we can reschedule or separate out the fix for #2727. Just wanted to remind you that this is waiting on attention.
@tdonohue: So if I remember correctly 😅 this PR did work before the latest Angular upgrade, so this fix should fix both issues on 7.6.2. So if you want to already merge this PR in order to fix #2724 on main
this can already be done, and then I could try and take a closer look later into which changes caused this fix to break on main
. I'm also going to try and take a closer look at this next week
Successfully created backport PR for dspace-7_x
:
- #3032
@alexandrevryghem : I've realized this PR appears to have had a side effect of increasing these statements in the UI logs:
The response for 'http://localhost:8080/server/api/authz/authorizations/search/object?uri=http%3A%2F%2Flocalhost%3A8080%2Fserver%2Fapi%2Fcore%2Fsites%2F6d65c6a2-3fe7-44dd-bacb-79271257c35d&feature=canSubmit'
has the self link 'http://localhost:8080/server/api/authz/authorizations/search/object?uri=http://localhost:8080/server/api/core/sites/6d65c6a2-3fe7-44dd-bacb-79271257c35d&feature=canSubmit'.
These don't match. This could mean there's an issue with the REST endpoint
The reason is that the UI is comparing one URL that has the RequestParam
values all encoded, to one which does NOT have those params encoded. If you look closely, in the statement above, the URLs are identical except for the encoded params.
Any thoughts on how we might solve this? In the meantime, I'll pull this into a new ticket