-
Notifications
You must be signed in to change notification settings - Fork 373
fix: Follow redirects during provenance check #11412
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
base: main
Are you sure you want to change the base?
fix: Follow redirects during provenance check #11412
Conversation
Signed-off-by: Mateusz Los <mateusz.los@extern.wenovate.de>
| val responseCode = requestSourceArtifact( | ||
| pkg, | ||
| "HEAD" | ||
| // some artifact are behind redirected urls, in case of 404 we need to fall back to GET to verify that. |
Check failure
Code scanning / detekt
Reports mis-indented code Error
| val responseCode = requestSourceArtifact( | ||
| pkg, | ||
| "HEAD" | ||
| // some artifact are behind redirected urls, in case of 404 we need to fall back to GET to verify that. |
Check failure
Code scanning / detekt
Detects trailing spaces Error
| val responseCode = requestSourceArtifact( | ||
| pkg, | ||
| "HEAD" | ||
| // some artifact are behind redirected urls, in case of 404 we need to fall back to GET to verify that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused about "redirects" in the commit message: By default, OkHttp is configured with followRedirects(true) and followSslRedirects(true). So it already follow redirects, also for HEAD requests.
So instead, this commit seems to do two different things:
- It falls back to a
GETrequest if theHEADrequests fails withHTTP_NOT_FOUND(for serves that do not implement HEAD correctly). - It regards
HTTP_MOVED_TEMPas a success, in case the requested resource was temporarily moved.
IMO both are independent changes which deserve separate commits with dedicated rationales in the commit message, an neither change is strictly related to following redirects properly.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11412 +/- ##
============================================
- Coverage 57.78% 57.75% -0.03%
+ Complexity 1713 1711 -2
============================================
Files 347 347
Lines 12904 12908 +4
Branches 1238 1239 +1
============================================
- Hits 7456 7455 -1
- Misses 5000 5006 +6
+ Partials 448 447 -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:
|
No description provided.