SEAB-7157: Fix TRS 500 #6121
SEAB-7157: Fix TRS 500 #6121
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## hotfix/1.17.1 #6121 +/- ##
===================================================
- Coverage 74.22% 74.18% -0.05%
+ Complexity 5663 5660 -3
===================================================
Files 389 389
Lines 20329 20332 +3
Branches 2100 2101 +1
===================================================
- Hits 15090 15083 -7
- Misses 4237 4246 +9
- Partials 1002 1003 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return url + selfPath.split(URLEncoder.encode(entry, StandardCharsets.UTF_8))[1]; | ||
| String[] splitPath = selfPath.split(URLEncoder.encode(entry, StandardCharsets.UTF_8)); | ||
| if (splitPath.length < 2) { | ||
| throw new CustomWebApplicationException("not found", HttpStatus.SC_NOT_FOUND); |
There was a problem hiding this comment.
a longer message or unique code may be useful for debugging for users who do double-encode
| @@ -817,7 +817,11 @@ private Response getFileByToolVersionID(String registryId, String versionIdParam | |||
| */ | |||
| private static String computeURLFromEntryAndRequestURI(String entry, String selfPath) { | |||
| String url = ToolsImplCommon.getUrlFromId(config, entry); | |||
| return url + selfPath.split(URLEncoder.encode(entry, StandardCharsets.UTF_8))[1]; | |||
| String[] splitPath = selfPath.split(URLEncoder.encode(entry, StandardCharsets.UTF_8)); | |||
There was a problem hiding this comment.
allergies messing with me, but it took me too long to realize that this was string splitting based on the encoded id itself
i.e. this gets you a split array with the first part of the url in the 0th index and any query parameters or other stuff that follows the ID in the second part
|
Description
Recently, we observed some 500s that are caused by some TRS path/url processing code going sideways because the original request was doubly-URL-encoded. For example, consider the request from the ticket:
The above triggers the 500 at this line
dockstore/dockstore-webservice/src/main/java/io/openapi/api/impl/ToolsApiServiceImpl.java
Line 820 in 48ff1a9
However, the proper singly-URL-encoded request:
works fine.
In this PR, we modify the malfunctioning function, so that if the split doesn't work, we return a
NOT_FOUNDresponse. IMHO, this is reasonable, because due to the double encoding, the file path is nonsensical, despite the fact that, somehow, the code successfully got to this spot.Review Instructions
The singly-encoded request should succeed.
The doubly-encoded request should fail with a 404.
Issue
https://ucsc-cgl.atlassian.net/browse/SEAB-7157
Security and Privacy
If there are any concerns that require extra attention from the security team, highlight them here and check the box when complete.
e.g. Does this change...
Please make sure that you've checked the following before submitting your pull request. Thanks!
mvn clean install@RolesAllowedannotation