Codeql improvements#919
Conversation
…tocols (https->http)
Summary - Extender code coverage reportSummary
Coveragecom/defold/extender - 28.2%
com/defold/extender/builders - 0%
com/defold/extender/cache - 32.4%
com/defold/extender/cache/info - 100%
com/defold/extender/log - 0%
com/defold/extender/metrics - 7.4%
com/defold/extender/process - 10.4%
com/defold/extender/remote - 4.9%
com/defold/extender/services - 43.8%
com/defold/extender/services/cocoapods - 51.8%
com/defold/extender/services/data - 80.7%
com/defold/extender/tracing - 18.7%
com/defold/extender/utils - 0%
|
| } | ||
|
|
||
| LOGGER.info("Using context for platform: {}", buildState.fullPlatform); | ||
| LOGGER.info("Using context for platform: {}", builder.platform); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a95d576746
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| return response; | ||
| } while(counter < maxRedirects); | ||
| throw new NullPointerException(String.format("Mac redirect count reached for request {}", url.toString())); |
There was a problem hiding this comment.
Throw IOException when redirect limit is exceeded
doRequestWithRedirects throws a NullPointerException when redirect handling fails, but its callers in SDK/mappings download paths only handle IOException. In a redirect loop or over-limit redirect chain, this runtime exception bypasses the existing retry/fallback branches and aborts the whole fetch instead of being handled like other transport failures. Returning an IOException here keeps failure behavior consistent with the surrounding error-handling logic.
Useful? React with 👍 / 👎.
| continue; | ||
| } | ||
| return response; | ||
| } while(counter < maxRedirects); |
There was a problem hiding this comment.
Enforce maxRedirectCount without off-by-one
The loop increments counter before making the request and then checks while (counter < maxRedirects), which means a configured limit of N only allows N-1 redirects (for example, maxRedirectCount=1 allows none). This can fail valid redirect chains exactly at the configured limit and makes the maxRedirectCount setting behave unexpectedly.
Useful? React with 👍 / 👎.
includeflagsdebugSourcePath