Fix URL signing api for URLs containing special characters#12435
Fix URL signing api for URLs containing special characters#12435ErykKul wants to merge 10 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
@ErykKul sorry, we bumped this to 6.12 given our capacity and how you said you're probably the only person using this API. 😄 |
| return error(Response.Status.INTERNAL_SERVER_ERROR, | ||
| "Requesting signed URLs requires a signing secret to be configured. Please set the dataverse.api.signing-secret JVM option."); | ||
| } | ||
|
|
There was a problem hiding this comment.
My concern with this being here is that it doesn't cover the URL signing when requesting a file download with a guestbook response. Those APIs still work without the secret. (See Access.java) These APIs all call UrlSignerUtil.signUrl, which should have this code to return an error if no signing-secret exists.
{
"status": "OK",
"data": {
"signedUrl": "http://localhost:8080/api/v1/access/dataset/4?gbrecs=true&gbrids=12&until=2026-06-15T20:36:32.302&user=usere7c863fd&method=GET&token=c99c3f9393be5ddedbd72829b6a8b29de3310b06f6692b77286351ba079c82af177c0e8c8df237afe7d6611b7c044e79cc7e402042bf07aa1f9cbbb9cf855298"
}
}
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
What this PR does / why we need it:
Bug fix; URL signing for URLs containing special characters broken in 6.10, this PR fixes the introduced bug.
Which issue(s) this PR closes:
Issue not created, bug fixed directly here.
Special notes for your reviewer:
Added tests to prevent regression. Also, API requires now the signing secret to be set, otherwise it will not work.
Suggestions on how to test this:
No special testing needed, the unit tests should be sufficient.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
Yes, included.
Additional documentation:
No