Ktor example #162
Ktor example #162
Conversation
|
Warning Rate limit exceeded@ashtanko has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
WalkthroughRefactors models into dev.shtanko.api.model, adds a Ktor-based GitHub client with suspend APIs and central JSON/constants, introduces multiple Ktor HttpClient plugins (ApiKey, Logging, Request/Response processing, Retry) and example, migrates dependencies to Version Catalog, and adds tests for the Ktor client and interceptors. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant KtorGitHubApi
participant HttpClient
participant Plugins
participant GitHubAPI as api.github.com
Caller->>KtorGitHubApi: getOrgRepos(org)
KtorGitHubApi->>HttpClient: prepare GET /orgs/{org}/repos?per_page=100
HttpClient->>Plugins: execute pipeline (ApiKey, Log, Processor, Retry)
Plugins->>HttpClient: forward to Engine
HttpClient->>GitHubAPI: HTTP request (Accept, Authorization)
GitHubAPI-->>HttpClient: 200 JSON [Repo]
HttpClient-->>KtorGitHubApi: body<List<Repo>>()
KtorGitHubApi-->>Caller: List<Repo>
sequenceDiagram
participant Client
participant ApiKey
participant Logger
participant Processor
participant Retry
participant Engine
participant Server
Client->>ApiKey: add header
ApiKey->>Logger: log request
Logger->>Processor: run requestProcessor
Processor->>Retry: send
Retry->>Engine: execute (with retries if needed)
Engine->>Server: HTTP request
Server-->>Engine: HTTP response
Engine-->>Retry: response
Retry->>Processor: run responseProcessor / decide retry
Processor->>Logger: log response
Logger->>Client: return final response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
src/main/kotlin/dev/shtanko/api/GitHubService.kt (1)
91-95: Incorrect Base64 encoding of auth token (produces garbage string)encode(...) returns ByteArray; toString(Charsets.UTF_8) is not valid here and yields something like [B@... Use encodeToString(..) instead.
httpClient: OkHttpClient = createHttpClient( format( AUTH_BASIC, - Base64.getEncoder().encode("$username:$password".toByteArray()).toString(Charsets.UTF_8), + Base64.getEncoder().encodeToString("$username:$password".toByteArray(Charsets.UTF_8)), ), ),
🧹 Nitpick comments (12)
src/main/kotlin/dev/shtanko/api/Dependencies.kt (1)
5-5: Good centralization of Json configignoreUnknownKeys=true is appropriate for GitHub payloads. Consider adding an explicit type for readability.
-internal val json = Json { ignoreUnknownKeys = true } +internal val json: Json = Json { ignoreUnknownKeys = true }src/main/kotlin/dev/shtanko/api/Constants.kt (1)
3-8: Consider adding User-Agent and reviewing Accept value
- GitHub requires a User-Agent header; add a constant to ensure it’s always set.
- Optionally plan migration to the newer Accept value application/vnd.github+json (keep current v3+json if tests depend on it).
internal const val BASE_URL = "https://api.github.com/" internal const val JSON_CONTENT_TYPE = "application/json" internal const val AUTH_HEADER = "Authorization" internal const val ACCEPT_HEADER = "Accept" internal const val ACCEPT_HEADER_VALUE = "application/vnd.github.v3+json" internal const val AUTH_BASIC = "Basic %s" +internal const val USER_AGENT = "User-Agent"If acceptable, later switch to the newer Accept:
- Keep: ACCEPT_HEADER_VALUE = "application/vnd.github.v3+json"
- Plan: ACCEPT_HEADER_VALUE = "application/vnd.github+json"
src/main/kotlin/dev/shtanko/api/KtorInterceptor.kt (3)
16-21: Prefer plugins over raw pipeline intercepts for header injectionUse DefaultRequest for stable header configuration, and drop the sendPipeline hack.
val client = HttpClient(CIO) { install(Logging) { level = LogLevel.ALL } + install(io.ktor.client.plugins.DefaultRequest) { + header("X-Custom-Header", "MyValue") + } } - client.sendPipeline.intercept(HttpSendPipeline.State) { request -> - println("Intercepted request to: $request") - // For example, add a custom header - context.headers.append("X-Custom-Header", "MyValue") - proceed() - }Also applies to: 22-27
29-33: Avoid duplicate request loggingThe Logging plugin already prints method and URL at LogLevel.ALL. This custom requestPipeline log is redundant.
- client.requestPipeline.intercept(HttpRequestPipeline.Before) { - println("Sending request to: ${context.url} ${context.method}") - proceed() - }
34-44: Use built-in retry plugin instead of manual resend logicReplace custom HttpSend intercept-based retry with HttpRequestRetry for clarity and to avoid subtle pipeline pitfalls.
- client.plugin(HttpSend).intercept { request -> - println("Request URL: ${request.url}") - println("Request method: ${request.method}") - println("Request headers: ${request.headers}") - val originalCall = execute(request) - if (originalCall.response.status.value !in 100..399) { - execute(request) - } else { - originalCall - } - } + client.install(io.ktor.client.plugins.HttpRequestRetry) { + maxRetries = 1 + retryIf { _, response -> response.status.value !in 100..399 } + exponentialDelay() + }src/main/kotlin/dev/shtanko/api/model/RequestData.kt (1)
5-10: Well-structured model with a security consideration.The data class is properly defined with @serializable annotation. However, consider the security implications of having a password field in a data class that could be logged or serialized.
Consider adding
@Transientor similar annotations to sensitive fields, or implement custom toString() to avoid accidentally exposing credentials in logs:@Serializable data class RequestData( val username: String, val password: String, val org: String, -) +) { + override fun toString(): String = "RequestData(username='$username', password='***', org='$org')" +}README.md (1)
25-29: Metrics updated — ensure consistency with generation pipelineNumbers look plausible given added modules. Please confirm these are auto-generated to avoid drift.
build.gradle.kts (1)
352-355: Use version catalog for logback to keep versions alignedDirect coordinate bypasses the catalog and can drift from test dependencies. Prefer the catalog alias if available.
- implementation("ch.qos.logback:logback-classic:1.4.11") + implementation(logback.classic)src/main/kotlin/dev/shtanko/api/KtorGithubService.kt (1)
25-28: Use Kotlin string templates for consistencyConsider using Kotlin string templates instead of
String.formatfor more idiomatic code:- val authToken = String.format( - AUTH_BASIC, - Base64.getEncoder().encodeToString("$username:$password".toByteArray(Charsets.UTF_8)), - ) + val authToken = "$AUTH_BASIC ${Base64.getEncoder().encodeToString("$username:$password".toByteArray(Charsets.UTF_8))}"gradle/libs.versions.toml (2)
36-43: Remove duplicate protobuf version definitionThe
protobufversion on line 42 duplicates the existingprotobufKotlin(line 36) andprotoc(line 37) versions. Consider using one of the existing version references to avoid maintenance overhead.-protobuf = "4.31.1"Then update the library references to use
version.ref = "protobufKotlin"orversion.ref = "protoc"instead.
89-90: Update version references after removing duplicateAfter removing the duplicate
protobufversion definition, update these libraries to use the existing version reference:-protobuf-java = { module = "com.google.protobuf:protobuf-java", version.ref = "protobuf" } -protobuf-kotlin-lite = { module = "com.google.protobuf:protobuf-kotlin-lite", version.ref = "protobuf" } +protobuf-java = { module = "com.google.protobuf:protobuf-java", version.ref = "protobufKotlin" } +protobuf-kotlin-lite = { module = "com.google.protobuf:protobuf-kotlin-lite", version.ref = "protobufKotlin" }api/Kotlin-Lab.api (1)
18340-18343: Consider removing main function from public APIThe
KtorInterceptorKtclass exposes amain()function in the public API. Main functions are typically entry points for applications, not libraries. If this is a demo or example, it should be moved to a separate example/demo module rather than being part of the public API surface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
README.md(1 hunks)api/Kotlin-Lab.api(4 hunks)build.gradle.kts(2 hunks)gradle/libs.versions.toml(2 hunks)src/main/kotlin/dev/shtanko/api/Constants.kt(1 hunks)src/main/kotlin/dev/shtanko/api/Dependencies.kt(1 hunks)src/main/kotlin/dev/shtanko/api/GitHubService.kt(1 hunks)src/main/kotlin/dev/shtanko/api/KtorGithubService.kt(1 hunks)src/main/kotlin/dev/shtanko/api/KtorInterceptor.kt(1 hunks)src/main/kotlin/dev/shtanko/api/contributors/Contributors.kt(1 hunks)src/main/kotlin/dev/shtanko/api/contributors/ContributorsImpl.kt(1 hunks)src/main/kotlin/dev/shtanko/api/contributors/Logger.kt(1 hunks)src/main/kotlin/dev/shtanko/api/model/Repo.kt(1 hunks)src/main/kotlin/dev/shtanko/api/model/RequestData.kt(1 hunks)src/main/kotlin/dev/shtanko/api/model/User.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/Aggregate.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/BlockingRequest.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/CallbackRequest.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsBackground.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsChannels.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsConcurrent.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsNotCancellable.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsProgress.kt(1 hunks)src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsSuspend.kt(1 hunks)src/test/kotlin/dev/shtanko/api/KtorGitHubApiTest.kt(1 hunks)src/test/kotlin/dev/shtanko/api/ModelsTest.kt(1 hunks)src/test/kotlin/dev/shtanko/api/contributors/MockGithubService.kt(1 hunks)src/test/kotlin/dev/shtanko/api/contributors/TestData.kt(1 hunks)src/test/kotlin/dev/shtanko/api/tasks/AggregationTest.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/kotlin/dev/shtanko/api/KtorGitHubApiTest.kt (2)
src/test/kotlin/dev/shtanko/concurrency/TestBase.kt (1)
runTest(67-103)src/main/kotlin/dev/shtanko/api/KtorGithubService.kt (1)
createKtorClient(20-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (24)
src/main/kotlin/dev/shtanko/api/model/Repo.kt (1)
5-9: Model looks correct and minimalFields match GitHub’s id and name. Serialization-ready and compatible with ignoreUnknownKeys.
src/test/kotlin/dev/shtanko/api/contributors/MockGithubService.kt (1)
20-21: Imports updated to new model package — LGTMImports now reference dev.shtanko.api.model.Repo/User; aligned with the refactor.
src/test/kotlin/dev/shtanko/api/ModelsTest.kt (1)
19-21: LGTM! Clean import reorganization.The import updates correctly reference the new model package structure, maintaining consistency across the codebase refactor.
src/test/kotlin/dev/shtanko/api/tasks/AggregationTest.kt (1)
19-19: LGTM! Consistent import update.Import path correctly updated to use the new model package location.
src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsBackground.kt (1)
20-20: LGTM! Import path correctly updated.Consistent with the model package reorganization.
src/test/kotlin/dev/shtanko/api/contributors/TestData.kt (1)
19-21: LGTM! Import updates align with package restructuring.All three model imports correctly reference the new dev.shtanko.api.model package.
src/main/kotlin/dev/shtanko/api/tasks/CallbackRequest.kt (1)
23-24: LGTM! Import paths correctly updated.Both RequestData and User imports now properly reference the model package.
src/main/kotlin/dev/shtanko/api/contributors/Logger.kt (1)
19-21: LGTM! Import reorganization properly implemented.All three model imports (Repo, RequestData, User) correctly updated to reference the new package structure.
src/main/kotlin/dev/shtanko/api/model/User.kt (1)
5-9: LGTM! Well-designed model class.Clean and appropriately structured data class with proper serialization support. The field names and types are intuitive and follow good naming conventions.
src/main/kotlin/dev/shtanko/api/contributors/ContributorsImpl.kt (1)
19-19: Import migration to model.User looks correctType resolution aligns with the PR-wide move to dev.shtanko.api.model. No functional impact here.
README.md (1)
35-40: Complexity report updatedCounts reflect modest growth; fine from doc perspective. No action needed if sourced from the same tool chain as before.
src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsNotCancellable.kt (1)
23-24: Import migration to model.RequestData and model.UserConsistent with the new model package; no behavior changes.
src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsSuspend.kt (1)
22-23: Import migration to model.RequestData and model.UserLooks good and keeps the public signature unchanged.
src/main/kotlin/dev/shtanko/api/tasks/BlockingRequest.kt (1)
22-23: Import migration to model.RequestData and model.UserBlocking variant still uses retrofit2.Response; OK to coexist alongside Ktor-based paths for demonstration/testing.
src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsConcurrent.kt (1)
22-23: Import migration to model.RequestData and model.UserConsistent with the rest of the PR; no functional change.
src/main/kotlin/dev/shtanko/api/contributors/Contributors.kt (1)
31-32: Import migration complete — no leftover old model references foundRan searches for
dev.shtanko.api.User,RequestData, andRepo; no occurrences remain in the codebase.src/main/kotlin/dev/shtanko/api/tasks/Aggregate.kt (1)
19-19: Aggregate extension migrated to model.User; doc wording tweak
- Import now targets dev.shtanko.api.model.User — consistent with the refactor.
- Minor doc fix reads better; no code changes to the algorithm.
Also applies to: 27-27
src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsProgress.kt (1)
22-23: Imports migrated to dev.shtanko.api.model — LGTMTypes now resolve from the model package; no behavioral changes.
src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsChannels.kt (1)
22-23: Imports migrated to dev.shtanko.api.model — LGTMSignature remains consistent; only type origins changed.
build.gradle.kts (1)
383-383: Add Ktor Mock engine for tests — LGTMThis enables deterministic client tests.
src/test/kotlin/dev/shtanko/api/KtorGitHubApiTest.kt (1)
84-90: Header assertions — LGTMValidates Accept and Basic auth headers from createKtorClient as intended.
gradle/libs.versions.toml (1)
39-39: Verify Ktor version and security advisoriesThe dependency in
gradle/libs.versions.tomlis currently pinned to:
- Line 39:
ktor = "3.2.3"Please confirm that:
- 3.2.3 is the latest stable Ktor client release.
- There are no known security vulnerabilities affecting 3.2.3.
You can verify this by:
- Checking Maven Central for
io.ktor:ktor-client-core(and related artifacts) to identify the newest release.- Reviewing the JetBrains/ktor GitHub releases and changelog for any security fixes.
- Searching NVD/CVE databases, Snyk, OSS Index, or GitHub Dependabot alerts for CVEs targeting version 3.2.3.
If you discover a newer, more secure version, please update this entry accordingly.
api/Kotlin-Lab.api (2)
18402-18403: LGTM!The logging functions have been properly updated to use the new model classes from
dev/shtanko/api/modelpackage, maintaining consistency with the refactoring.
18549-18578: LGTM!All contributor loading functions have been consistently updated to use the new
RequestDatamodel class, maintaining API consistency across the different loading strategies (blocking, callbacks, background, channels, concurrent, suspend, etc.).
| public final fun getPassword ()Ljava/lang/String; | ||
| public final fun getUsername ()Ljava/lang/String; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security concern: Password field in RequestData
The RequestData class exposes a getPassword() method, indicating it stores passwords. This raises security concerns:
- Passwords should not be stored in plain text
- If this is for API authentication, consider using tokens or OAuth instead
- Ensure passwords are never logged or serialized to persistent storage
🏁 Script executed:
#!/bin/bash
# Check how the password field is used in the codebase
echo "Checking usage of RequestData.password field..."
# Search for password usage in RequestData
ast-grep --pattern 'getPassword()'
# Check if passwords are being logged
rg -A 3 -B 3 'logRepos.*RequestData|RequestData.*log'
# Check for any password logging patterns
rg -i 'log.*password|password.*log'Length of output: 1974
Security improvement: redact passwords in RequestData
We’ve verified that:
Logger.logRepos(req, response)(src/main/kotlin/dev/shtanko/api/contributors/Logger.kt) does not log the password field- No other
log.*passwordpatterns exist in the codebase
However, RequestData still exposes a plain-text password via its generated getPassword() and default toString(), making it easy to leak or accidentally serialize sensitive data.
Please address the following:
- api/Kotlin-Lab.api (lines 18488–18489):
getPassword()remains publicly exposed - Override
toString()inRequestData(src/main/kotlin/dev/shtanko/api/model/RequestData.kt) to redact or omit the password - Annotate the
passwordproperty with@kotlinx.serialization.Transientor use a custom serializer to prevent accidental serialization/logging - Consider migrating to token-based authentication (e.g., OAuth or HTTP Authorization header) rather than sending raw passwords in JSON
- Avoid storing passwords in immutable
String—useCharArrayor a secure credential store where feasible
🤖 Prompt for AI Agents
api/Kotlin-Lab.api around lines 18488–18489: the generated API exposes
getPassword() and the RequestData default toString(), risking plain-text
password leaks; in src/main/kotlin/dev/shtanko/api/model/RequestData.kt override
toString() to redact or omit the password field, annotate the password property
with @kotlinx.serialization.Transient or apply a custom serializer so it is not
serialized, remove or make the getPassword accessor non-public (or avoid
exposing a String getter) and prefer storing credentials as a CharArray or using
a secure credential store, and update authentication flow to use token-based
auth (Authorization header/OAuth) instead of raw passwords in JSON.
| ktor.apply { | ||
| client.apply { | ||
| implementation(client.core) | ||
| implementation(client.okhttp) | ||
| implementation(client.content.negotiation) | ||
| implementation(client.logging) | ||
| implementation(client.cio) | ||
| } | ||
| implementation(ktor.serialization.kotlinx.json) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid including multiple Ktor client engines unless required
Both CIO and OkHttp engines are added. This increases classpath size and can cause confusion. Prefer a single engine (CIO is already the default in createKtorClient).
Apply one of the diffs to keep a single engine:
Option A (keep CIO):
client.apply {
implementation(client.core)
- implementation(client.okhttp)
implementation(client.content.negotiation)
implementation(client.logging)
implementation(client.cio)
}Option B (keep OkHttp):
client.apply {
implementation(client.core)
implementation(client.okhttp)
implementation(client.content.negotiation)
implementation(client.logging)
- implementation(client.cio)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ktor.apply { | |
| client.apply { | |
| implementation(client.core) | |
| implementation(client.okhttp) | |
| implementation(client.content.negotiation) | |
| implementation(client.logging) | |
| implementation(client.cio) | |
| } | |
| implementation(ktor.serialization.kotlinx.json) | |
| } | |
| ktor.apply { | |
| client.apply { | |
| implementation(client.core) | |
| implementation(client.content.negotiation) | |
| implementation(client.logging) | |
| implementation(client.cio) | |
| } | |
| implementation(ktor.serialization.kotlinx.json) | |
| } |
🤖 Prompt for AI Agents
In build.gradle.kts around lines 342 to 351, the Ktor client is including both
CIO and OkHttp engines; remove the redundant engine to avoid classpath
bloat—prefer keeping CIO (the default). Edit the block to delete the
implementation(client.okhttp) line so only implementation(client.cio) remains
(or if you intentionally prefer OkHttp, delete implementation(client.cio)
instead); keep all other client implementations unchanged.
| fun createKtorClient( | ||
| username: String, | ||
| password: String, | ||
| engine: HttpClientEngine = CIO.create(), | ||
| ): HttpClient { | ||
| val authToken = String.format( | ||
| AUTH_BASIC, | ||
| Base64.getEncoder().encodeToString("$username:$password".toByteArray(Charsets.UTF_8)), | ||
| ) | ||
|
|
||
| return HttpClient(engine) { | ||
| install(ContentNegotiation) { | ||
| json(json) | ||
| } | ||
| install(Logging) { | ||
| level = LogLevel.INFO | ||
| } | ||
| defaultRequest { | ||
| header(ACCEPT_HEADER, ACCEPT_HEADER_VALUE) | ||
| header(HttpHeaders.Authorization, authToken) | ||
| url(BASE_URL) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider security improvements for authentication and logging
The current implementation has security considerations:
- Basic authentication transmits credentials with every request. Consider using GitHub Personal Access Tokens or OAuth tokens instead.
LogLevel.INFOmight expose sensitive data in logs. Consider usingLogLevel.HEADERSorLogLevel.NONEin production, or implement custom sanitization.
🏁 Script executed:
#!/bin/bash
# Verify that the required constants are defined in the codebase
echo "Checking for AUTH_BASIC constant..."
rg "AUTH_BASIC\s*=" -A 2
echo "Checking for ACCEPT_HEADER constant..."
rg "ACCEPT_HEADER\s*=" -A 2
echo "Checking for ACCEPT_HEADER_VALUE constant..."
rg "ACCEPT_HEADER_VALUE\s*=" -A 2
echo "Checking for BASE_URL constant..."
rg "BASE_URL\s*=" -A 2
echo "Checking for json instance in Dependencies..."
rg "val json\s*=" src/main/kotlin/dev/shtanko/api/Dependencies.kt -A 3Length of output: 1865
Enhance authentication security and tighten logging
The required constants (AUTH_BASIC, ACCEPT_HEADER, ACCEPT_HEADER_VALUE, BASE_URL, and the json instance) have been confirmed in Constants.kt and Dependencies.kt. To harden the client:
-
Move away from Basic auth
Sending raw credentials on every request increases risk. Prefer a GitHub Personal Access Token (PAT) or OAuth flow, passed in the Authorization header (e.g.token $PAT), to avoid embedding username/password in your client. -
Reduce log verbosity
UsingLogLevel.INFOmay inadvertently log headers or payloads containing sensitive information. In production, switch toLogLevel.HEADERS,LogLevel.NONE, or implement a custom logger that redacts theAuthorizationheader. -
Optional Kotlin refinements
- Instead of
String.format(AUTH_BASIC, encoded), you can use Kotlin string templates:
header(HttpHeaders.Authorization, "Basic $encoded") - You already explicitly set
Charsets.UTF_8, which is good practice.
- Instead of
Please address these security recommendations before merging.
🤖 Prompt for AI Agents
In src/main/kotlin/dev/shtanko/api/KtorGithubService.kt around lines 20 to 43,
replace passing raw username/password with a token-based approach and reduce
logging: change the client to accept a single secret token (or an optional token
fallback) and set the Authorization header to use the token scheme (e.g., "token
$PAT" or "Bearer $PAT") instead of Basic auth; lower the Logging level to
HEADERS or NONE (or implement a custom logger that redacts the Authorization
header) to avoid leaking credentials; optionally simplify the header
construction using Kotlin string templates when forming the Authorization
header.
| suspend fun getOrgRepos(org: String): List<Repo> { | ||
| return client.get("orgs/$org/repos") { | ||
| parameter("per_page", 100) | ||
| }.body() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and pagination support for getOrgRepos
The current implementation has limitations:
- No error handling for HTTP failures or deserialization errors
- GitHub API returns maximum 100 items per page - large organizations may have more repositories
- No input validation for the
orgparameter
Consider implementing pagination and error handling:
suspend fun getOrgRepos(org: String): List<Repo> {
require(org.isNotBlank()) { "Organization name cannot be blank" }
return try {
// TODO: Implement pagination to fetch all repositories
client.get("orgs/$org/repos") {
parameter("per_page", 100)
// parameter("page", pageNumber) for pagination
}.body()
} catch (e: Exception) {
// Log error and handle appropriately
throw GitHubApiException("Failed to fetch repositories for org: $org", e)
}
}🤖 Prompt for AI Agents
In src/main/kotlin/dev/shtanko/api/KtorGithubService.kt around lines 47 to 51,
validate the org input (e.g., require(org.isNotBlank())), implement pagination
by repeatedly requesting "orgs/$org/repos" with parameter per_page=100 and
page=N, accumulating bodies until an empty page (or until Link header indicates
no next page), and handle HTTP/deserialization errors by catching exceptions,
logging them, and rethrowing a domain-specific exception (e.g.,
GitHubApiException) that wraps the original error; ensure resources or client
calls are used correctly and avoid infinite loops by incrementing the page
counter and breaking when no more items are returned.
| suspend fun getRepoContributors(owner: String, repo: String): List<User> { | ||
| return client.get("repos/$owner/$repo/contributors") { | ||
| parameter("per_page", 100) | ||
| }.body() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and pagination support for getRepoContributors
Similar to getOrgRepos, this function needs:
- Error handling for HTTP and deserialization failures
- Pagination support (repositories can have more than 100 contributors)
- Input validation for
ownerandrepoparameters
| client.plugin(HttpSend).intercept { request -> | ||
| println("Request URL: ${request.url}") | ||
| println("Request method: ${request.method}") | ||
| println("Request headers: ${request.headers}") | ||
| val originalCall = execute(request) | ||
| if (originalCall.response.status.value !in 100..399) { | ||
| execute(request) | ||
| } else { | ||
| originalCall | ||
| } | ||
| } |
There was a problem hiding this comment.
Do not log sensitive headers (Authorization/PII)
Logging the entire header set can leak credentials. Mask or drop sensitive headers before printing.
- println("Request headers: ${request.headers}")
+ val safeHeaders = request.headers.entries().associate { (k, v) ->
+ if (k.equals("Authorization", ignoreCase = true)) k to listOf("***") else k to v
+ }
+ println("Request headers: $safeHeaders")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client.plugin(HttpSend).intercept { request -> | |
| println("Request URL: ${request.url}") | |
| println("Request method: ${request.method}") | |
| println("Request headers: ${request.headers}") | |
| val originalCall = execute(request) | |
| if (originalCall.response.status.value !in 100..399) { | |
| execute(request) | |
| } else { | |
| originalCall | |
| } | |
| } | |
| client.plugin(HttpSend).intercept { request -> | |
| println("Request URL: ${request.url}") | |
| println("Request method: ${request.method}") | |
| val safeHeaders = request.headers.entries().associate { (k, v) -> | |
| if (k.equals("Authorization", ignoreCase = true)) { | |
| k to listOf("***") | |
| } else { | |
| k to v | |
| } | |
| } | |
| println("Request headers: $safeHeaders") | |
| val originalCall = execute(request) | |
| if (originalCall.response.status.value !in 100..399) { | |
| execute(request) | |
| } else { | |
| originalCall | |
| } | |
| } |
| val response = client.get("https://httpbin.org/get") | ||
| println("Response status: ${response.status}") | ||
|
|
||
| client.close() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Close the client reliably
Ensure the client is closed on exceptions as well.
- val response = client.get("https://httpbin.org/get")
- println("Response status: ${response.status}")
-
- client.close()
+ try {
+ val response = client.get("https://httpbin.org/get")
+ println("Response status: ${response.status}")
+ } finally {
+ client.close()
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val response = client.get("https://httpbin.org/get") | |
| println("Response status: ${response.status}") | |
| client.close() | |
| } | |
| try { | |
| val response = client.get("https://httpbin.org/get") | |
| println("Response status: ${response.status}") | |
| } finally { | |
| client.close() | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/dev/shtanko/api/KtorInterceptor.kt around lines 46 to 50, the
HttpClient is closed only on the normal path, so it will leak if an exception
occurs; wrap the request/response logic so the client is always closed (for
example use a use {} block on the client or surround the request with try { ...
} finally { client.close() }) to guarantee client.close() runs on exceptions as
well.
| import io.ktor.http.headersOf | ||
| import io.ktor.serialization.kotlinx.json.json | ||
| import kotlinx.coroutines.test.runTest | ||
| import kotlinx.serialization.json.Json |
There was a problem hiding this comment.
Missing import for Json.encodeToString leads to compile error
Add the extension import so encodeToString resolves.
import kotlinx.serialization.json.Json
+import kotlinx.serialization.encodeToString📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import kotlinx.serialization.json.Json | |
| import kotlinx.serialization.json.Json | |
| import kotlinx.serialization.encodeToString |
🤖 Prompt for AI Agents
In src/test/kotlin/dev/shtanko/api/KtorGitHubApiTest.kt around line 16, the call
to Json.encodeToString fails to resolve because the extension import is missing;
add the top-level extension import `kotlinx.serialization.encodeToString` to the
file so the Json.encodeToString extension function is available and the test
compiles.
# Conflicts: # build.gradle.kts
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
gradle/libs.versions.toml (1)
119-119: KAPT plugin id is incorrect and version should align to Kotlin.Gradle plugin id must be
org.jetbrains.kotlin.kapt. Usingid = "kapt"will fail. Also, kapt plugin version should match the Kotlin version.-kapt = { id = "kapt", version.ref = "kapt" } +kapt = { id = "org.jetbrains.kotlin.kapt", version.ref = "kotlin" }Additionally, remove the standalone
kaptversion from [versions] and rely on Kotlin:- kapt = "2.1.0" # https://kotlinlang.org/docs/kapt.html
🧹 Nitpick comments (4)
gradle/libs.versions.toml (4)
39-43: Reduce duplicate protobuf version keys.You now have protobufKotlin/protoc/protobuf all at 4.31.1. Consider a single
protobufversion key to simplify upgrades.Example minimal change: point
protobuf-kotlinandprotobuf-protoctoversion.ref = "protobuf"and drop the extra version entries.
74-80: Avoid pulling multiple Ktor client engines unless needed.Including both CIO and OkHttp increases size and may cause accidental engine selection. Keep only the engine(s) you use.
-ktor-client-okhttp = { module = "io.ktor:ktor-client-okhttp", version.ref = "ktor" } -ktor-client-cio = { module = "io.ktor:ktor-client-cio", version.ref = "ktor" } +ktor-client-okhttp = { module = "io.ktor:ktor-client-okhttp", version.ref = "ktor" } +# Remove the unused engine to avoid bloat; keep only one.If you actually use both in different modules, consider moving one to that specific module’s catalog or define a bundle per module.
88-91: Align protobuf libs via a single version/BOM.Manually pinning
protobuf-javaandprotobuf-kotlin-liteis OK but easy to drift. Prefer a single version key or adopt the protobuf BOM if available for your toolchain.Option A (single key): point both to
version.ref = "protobuf"and remove other protobuf version aliases.
Option B (BOM): introduceprotobuf-bomand import it viaplatform(...)in module build files; then drop explicit versions here.
92-94: Use the gRPC BOM to keep modules in lockstep.
grpc-stubandgrpc-protobufshould stay aligned; the BOM simplifies this and eases upgrades.
- Add library alias:
grpc-bom = { module = "io.grpc:grpc-bom", version.ref = "grpc" }- In modules:
implementation(platform(libs.grpc.bom))then declaregrpc-stubandgrpc-protobufwithout versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gradle/libs.versions.toml(4 hunks)
🔇 Additional comments (3)
gradle/libs.versions.toml (3)
82-84: JOL addition looks good.
116-116: Compatibility confirmed: Kotlin 2.2.0 + kotlinx-serialization-json 1.9.0 + Ktor 3.2.x
The Kotlin serialization plugin (2.2.0) and kotlinx-serialization-json 1.9.0 are officially compatible on Kotlin 2.2, and Ktor 3.2.x fully supports kotlinx-serialization-based content negotiation. No changes required.• File gradle/libs.versions.toml, line 116 – serialization plugin version.ref = "kotlin" remains correct.
4-4: Verify cross-compatibility of Kotlin 2.2.0, Retrofit 3.0.0 & OkHttp 5.1.0Based on upstream guidance, here’s what you need to confirm:
Kotlin 2.2.0
• Fully supported by Ktor 3.2.x (prefer 3.2.2+) on JVM and MPP.
• kotlinx-coroutines 1.10.x works on JVM (Kotlin ≥ 2.0) and on Native only if Kotlin ≥ 2.1 (2.2 is fine).Retrofit 3.0.0 + kotlinx-serialization converter 3.0.0
• Officially built against OkHttp 4.12.x.
• OkHttp 5.x introduces breaking/packaging changes—Retrofit 3 artifacts were not compiled for it.What to do next:
If you want a fully supported stack, align your versions in gradle/libs.versions.toml to:
• okhttp = "4.12.0"
• retrofit = "3.0.0"
• converter-kotlinx-serialization = "3.0.0"
• kotlinx-serialization-json = "1.8.1"If you must keep OkHttp 5.1.0, add integration tests around every Retrofit call and watch for binary/API/runtime issues. Be prepared to roll back or wait for a Retrofit build targeting OkHttp 5.
Please review and confirm your chosen version strategy.
| # Retrofit kotlinx.serialization converter | ||
| retrofit2-kotlinx-serialization-converter = { module = "com.jakewharton.retrofit:retrofit2-kotlinx-serialization-converter", version.ref = "retrofit-kotlinx-serialization" } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Retrofit 3 likely incompatible with Jake Wharton’s retrofit2-kotlinx-serialization converter.
The artifact targets Retrofit 2.x (note retrofit2- prefix) and may not support 3.x.
Preferred fix: switch to Square’s official converter aligned with Retrofit 3 coordinates.
-# Retrofit kotlinx.serialization converter
-retrofit2-kotlinx-serialization-converter = { module = "com.jakewharton.retrofit:retrofit2-kotlinx-serialization-converter", version.ref = "retrofit-kotlinx-serialization" }
+# Retrofit kotlinx.serialization converter (align version with Retrofit)
+retrofit-kotlinx-serialization = { module = "com.squareup.retrofit2:converter-kotlinx-serialization", version.ref = "retrofit" }If unsure, please check the Retrofit 3 docs for the recommended converter artifact and version:
🌐 Web query:
Retrofit 3 official kotlinx-serialization converter artifact and version alignment guidance
💡 Result:
Short answer
- Official Retrofit 3 kotlinx‑serialization converter artifact:
com.squareup.retrofit2:converter-kotlinx-serialization:3.0.0. (mvnrepository.com)
What this means and version-alignment rules
- The kotlinx‑serialization converter is now distributed as a first‑party Retrofit module (group com.squareup.retrofit2) and its versions follow Retrofit’s release stream (e.g., converter 3.0.0 with retrofit 3.0.0). Use the converter version that matches the Retrofit major/minor you use, or manage all Retrofit modules together with the retrofit-bom. (mvnrepository.com)
- The converter has a runtime dependency on kotlinx.serialization runtime libraries (kotlinx-serialization-core / kotlinx-serialization-json) and on Kotlin stdlib. Make sure your kotlinx-serialization runtime and the Kotlin compiler/plugin you use are compatible (use the kotlinx.serialization series recommended for your Kotlin version). The Kotlin docs and kotlinx.serialization releases are the authoritative compatibility source. (kotlinlang.org, github.com)
- If you previously used Jake Wharton’s retrofit2-kotlinx-serialization-converter, migrate to the first‑party artifact (the old repo is archived). (github.com)
Recommended Gradle setups (examples)
-
Using the Retrofit BOM (recommended to keep retrofit + converters aligned)
-
Kotlin DSL (build.gradle.kts):
implementation(platform("com.squareup.retrofit2:retrofit-bom:3.0.0"))
implementation("com.squareup.retrofit2:retrofit")
implementation("com.squareup.retrofit2:converter-kotlinx-serialization") -
Groovy:
implementation platform('com.squareup.retrofit2:retrofit-bom:3.0.0')
implementation 'com.squareup.retrofit2:retrofit'
implementation 'com.squareup.retrofit2:converter-kotlinx-serialization'
The BOM pins the Retrofit main artifact and its official converters to matching versions. (mvnrepository.com)
-
-
Add kotlinx.serialization runtime and Kotlin serialization plugin (example targets that work with Retrofit 3 era):
- Kotlin DSL (minimum required lines):
plugins {
kotlin("jvm") version "2.1.20" // or your project Kotlin version
kotlin("plugin.serialization") version "2.1.20"
}
dependencies {
implementation("org.jetbrains.kotlinx:kotlinx-serialization-json:1.9.0")
} - Check the kotlinx.serialization release notes / documentation to pick the right library version for your Kotlin version. (Examples shown here reflect the 1.8/1.9 series available in 2024–2025.) (github.com, kotlinlang.org)
- Kotlin DSL (minimum required lines):
Usage example (Kotlin)
- Typical builder:
val contentType = "application/json; charset=UTF-8".toMediaType()
val json = Json { ignoreUnknownKeys = true } // configure as needed
val retrofit = Retrofit.Builder()
.baseUrl("https://api.example.com/")
.addConverterFactory(json.asConverterFactory(contentType))
.build()
Notes / gotchas
- Order of converters matters: the kotlinx‑serialization converter usually claims many types; if mixing converters, add other converters first and kotlinx‑serialization last if you want the others to get first shot at compatible types. (github.com)
- Align Kotlin compiler (plugin) version, Kotlin stdlib, and kotlinx-serialization runtime. Mismatches produce compile-time or runtime errors. Consult the kotlinx.serialization release notes for compatibility (e.g., 1.8.x / 1.9.x series align with Kotlin 2.0–2.2 era releases). (github.com, kotlinlang.org)
- If you want to pin everything and avoid manual cross-checks, use retrofit-bom for retrofit modules and follow the Kotlin docs for the serialization plugin/runtime versions. (mvnrepository.com, kotlinlang.org)
If you want, I can:
- produce an exact build.gradle(.kts) snippet for your project (tell me your Kotlin and Retrofit versions), or
- check the currently published latest versions in your build environment right now and produce a copy‑paste dependency block.
Citations:
- 1: https://mvnrepository.com/artifact/com.squareup.retrofit2/converter-kotlinx-serialization/3.0.0?utm_source=chatgpt.com
- 2: https://mvnrepository.com/artifact/com.squareup.retrofit2/retrofit/3.0.0?utm_source=chatgpt.com
- 3: https://kotlinlang.org/docs/serialization.html?utm_source=chatgpt.com
- 4: https://github.com/Kotlin/kotlinx.serialization/releases?utm_source=chatgpt.com
- 5: https://github.com/JakeWharton/retrofit2-kotlinx-serialization-converter?utm_source=chatgpt.com
- 6: https://mvnrepository.com/artifact/com.squareup.retrofit2/retrofit-bom/3.0.0?utm_source=chatgpt.com
- 7: https://github.com/Kotlin/kotlinx.serialization/releases?utm_source=chatgpt.com
- 8: https://kotlinlang.org/docs/serialization.html?utm_source=chatgpt.com
- 9: https://github.com/JakeWharton/retrofit2-kotlinx-serialization-converter?utm_source=chatgpt.com
- 10: https://github.com/Kotlin/kotlinx.serialization/releases?utm_source=chatgpt.com
- 11: https://kotlinlang.org/docs/serialization.html?utm_source=chatgpt.com
- 12: https://mvnrepository.com/artifact/com.squareup.retrofit2/retrofit-bom/3.0.0?utm_source=chatgpt.com
- 13: https://kotlinlang.org/docs/serialization.html?utm_source=chatgpt.com
Align with Retrofit 3’s first-party kotlinx-serialization converter
The Jake Wharton retrofit2-kotlinx-serialization-converter targets Retrofit 2.x and is archived; Retrofit 3 provides its own converter under com.squareup.retrofit2. Update your version catalog to use the official module and keep versions in sync (via the retrofit version or the Retrofit BOM).
• In gradle/libs.versions.toml (around lines 85–86):
-# Retrofit kotlinx.serialization converter
-retrofit2-kotlinx-serialization-converter = { module = "com.jakewharton.retrofit:retrofit2-kotlinx-serialization-converter", version.ref = "retrofit-kotlinx-serialization" }
+# Retrofit kotlinx.serialization converter (official Retrofit 3)
+converter-kotlinx-serialization = { module = "com.squareup.retrofit2:converter-kotlinx-serialization", version.ref = "retrofit" }Optional: Add the Retrofit BOM to your Gradle setup to lock all Retrofit artifacts (core + converters) to the same 3.x version:
// build.gradle.kts
implementation(platform("com.squareup.retrofit2:retrofit-bom:3.0.0"))
implementation("com.squareup.retrofit2:retrofit")
implementation("com.squareup.retrofit2:converter-kotlinx-serialization")🤖 Prompt for AI Agents
In gradle/libs.versions.toml around lines 85–86, the catalog currently
references Jake Wharton’s archived retrofit2-kotlinx-serialization-converter
which targets Retrofit 2; replace that entry with the official Retrofit 3
first‑party converter module
(com.squareup.retrofit2:converter-kotlinx-serialization) and point its version
to the same retrofit version reference (or remove the explicit version and rely
on the Retrofit BOM), and optionally add instructions to use the Retrofit BOM in
build.gradle.kts to lock core + converter to the same 3.x version.
… into feature/ktor_example
# Conflicts: # gradle/libs.versions.toml
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (4)
gradle/libs.versions.toml (1)
85-87: Replace Jake Wharton’s converter with Retrofit 3’s official kotlinx-serialization converterThe Jake Wharton artifact targets Retrofit 2.x and is archived. With Retrofit = 3.0.0 in this catalog, this will break or force a downgrade. Switch to the first‑party Retrofit 3 converter and align its version to Retrofit’s.
Apply this diff:
-# Retrofit kotlinx.serialization converter -retrofit2-kotlinx-serialization-converter = { module = "com.jakewharton.retrofit:retrofit2-kotlinx-serialization-converter", version.ref = "retrofit-kotlinx-serialization" } +# Retrofit kotlinx.serialization converter (official Retrofit 3) +converter-kotlinx-serialization = { module = "com.squareup.retrofit2:converter-kotlinx-serialization", version.ref = "retrofit" }Optional (keeps all Retrofit artifacts aligned):
- Add a BOM entry in the catalog and use it in build scripts:
- Library entry:
retrofit-bom = { module = "com.squareup.retrofit2:retrofit-bom", version.ref = "retrofit" }- build.gradle(.kts):
implementation(platform(libs.retrofit.bom)) implementation(libs.retrofit) implementation(libs.converter.kotlinx.serialization)api/Kotlin-Lab.api (3)
18566-18580: Plain-text password in RequestData (getter + default toString) risks leaks; redact and avoid serializationThis mirrors a previous review: getPassword() and a data-class toString() expose sensitive data. Replace with a safer shape, redact toString(), and prefer HTTP Authorization over JSON passwords.
Minimal mitigation (RequestData.kt):
-@Serializable -data class RequestData( - val org: String, - val username: String, - val password: String, -) +@Serializable +data class RequestData( + val org: String, + val username: String, + @Transient val password: String? = null, // not serialized +) { + override fun toString(): String = + "RequestData(org=$org, username=$username, password=**redacted**)" +}Preferred redesign:
- Remove password from RequestData entirely; introduce Credentials carried via Ktor Auth:
- Basic: Auth.basic { credentials { BasicAuthCredentials(username, password) } }
- Bearer: Auth.bearer { loadTokens { BearerTokens(apiKey, "") } }
If you must hold secrets in memory, prefer CharArray and clear it after use.
18582-18595: Serializer still includes password; exclude or implement a custom serializerGiven RequestData$$serializer exists, the password will be serialized today. Either mark the field @transient or provide a custom serializer that omits/redacts it.
Custom serializer sketch:
@Serializable(with = SafeRequestDataSerializer::class) data class RequestData(val org: String, val username: String, @Transient val password: String? = null) object SafeRequestDataSerializer : KSerializer<RequestData> { override val descriptor = buildClassSerialDescriptor("RequestData") { element<String>("org"); element<String>("username") // Intentionally no "password" } override fun serialize(encoder: Encoder, value: RequestData) { encoder.encodeStructure(descriptor) { encodeStringElement(descriptor, 0, value.org) encodeStringElement(descriptor, 1, value.username) } } override fun deserialize(decoder: Decoder): RequestData { var org = ""; var username = "" decoder.decodeStructure(descriptor) { loop@ while (true) { when (val i = decodeElementIndex(descriptor)) { 0 -> org = decodeStringElement(descriptor, 0) 1 -> username = decodeStringElement(descriptor, 1) CompositeDecoder.DECODE_DONE -> break@loop else -> skipElement() } } } return RequestData(org, username) // password stays null } }
18637-18667: Avoid passing RequestData (with password) across tasks; split params and credentialsThese task APIs propagate RequestData throughout the app surface, increasing blast radius for credential leaks. Split into immutable “params” and a separate credential source (Ktor Auth).
Recommended migration pattern:
- Introduce:
- data class RequestParams(val org: String, val username: String)
- Use Ktor Auth for credentials, not function params
- Keep the old signatures temporarily with @deprecated and forward to new ones.
Example for one function:
- fun loadContributorsBlocking(service: GitHubService, req: RequestData): List<User> + fun loadContributorsBlocking(service: GitHubService, params: RequestParams): List<User>I can prepare a follow-up patch updating all task entry points and wiring Ktor Auth. Want me to draft it?
🧹 Nitpick comments (16)
gradle/libs.versions.toml (2)
74-81: Ktor client entries look good; consider a bundle for easy reuseThe set of Ktor modules is coherent for a typical client stack. To keep build files DRY across modules, consider defining a bundle in the catalog for common Ktor deps.
Example bundle (add to [bundles] in this file):
[bundles] ktor-client = [ "ktor-client-core", "ktor-client-content-negotiation", "ktor-serialization-kotlinx-json", "ktor-client-logging" ]If you only use one engine per module, avoid adding both CIO and OkHttp at the same time to keep the dependency graph lean.
92-94: gRPC entries OK; optionally adopt gRPC BOM and Kotlin stubThe stub/protobuf pair is aligned via the grpc version alias. For larger graphs, consider the gRPC BOM to guarantee transitive alignment across all grpc-* artifacts. If you plan coroutine‑first clients, also add grpc-kotlin-stub.
- Optional catalog additions:
grpc-bom = { module = "io.grpc:grpc-bom", version.ref = "grpc" } grpc-kotlin-stub = { module = "io.grpc:grpc-kotlin-stub", version.ref = "grpc" }- build.gradle(.kts):
implementation(platform(libs.grpc.bom)) implementation(libs.grpc.stub) implementation(libs.grpc.protobuf) // optional for Kotlin: implementation(libs.grpc.kotlin.stub)If you intend to use grpc-kotlin-stub, confirm the generated sources (protoc plugins) are configured accordingly in your protobuf Gradle plugin setup.
src/main/kotlin/dev/shtanko/api/ktor/RequestResponseProcessorInterceptor.kt (1)
11-14: Consider making requestProcessor suspend for parity and flexibility.Symmetric suspend handlers give you freedom to do asynchronous work in both hooks (e.g., read from storage, metrics).
Suggested change:
class RequestResponseProcessorInterceptor private constructor( - private val requestProcessor: (HttpRequestBuilder) -> Unit, + private val requestProcessor: suspend (HttpRequestBuilder) -> Unit, private val responseProcessor: suspend (HttpResponse) -> Unit ) { class Config { - var requestProcessor: (HttpRequestBuilder) -> Unit = { _ -> } + var requestProcessor: suspend (HttpRequestBuilder) -> Unit = { _ -> } var responseProcessor: suspend (HttpResponse) -> Unit = { _: HttpResponse -> } } ... - scope.requestPipeline.intercept(HttpRequestPipeline.Before) { - plugin.requestProcessor(context) - } + scope.requestPipeline.intercept(HttpRequestPipeline.Before) { + plugin.requestProcessor(context) + }Also applies to: 16-19, 31-33
src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt (2)
37-64: Optionally restrict retries to idempotent/safe HTTP methods.Minimize side effects by retrying only for GET/HEAD/PUT/DELETE/OPTIONS/TRACE unless explicitly configured otherwise.
Would you like a follow-up patch that gates retries by request.method with an allowlist and/or per-request override via attributes?
3-10: Minor: Logging via println in a library plugin.Route logs through a logger (e.g., KtorSimpleLogger or your project’s logging abstraction) and allow consumers to configure log level/output. Keep emojis out of machine-parsed logs.
If you want, I can wire a minimal logger wrapper and replace the println calls across interceptors.
api/Kotlin-Lab.api (2)
18340-18343: Example main should be isolated from production runtimeEnsure KtorInterceptorExampleKt.main is not packaged/executed in production (move to a samples sourceSet/module or guard with an environment flag). This avoids accidental execution in CI/CD or consumer environments.
18493-18512: Request/Response processors: guard against exceptions and control dispatchingProcessor hooks can destabilize the pipeline if they throw. Wrap with try/catch and allow dispatching to a specified CoroutineContext to avoid blocking IO threads.
Sketch:
class RequestResponseProcessorInterceptor { class Config { var requestProcessor: (HttpRequestBuilder) -> Unit = {} var responseProcessor: suspend (HttpResponse) -> Unit = {} var context: CoroutineContext = Dispatchers.Default } // In install: scope.sendPipeline.intercept(HttpSendPipeline.Monitoring) { try { config.requestProcessor(context) } catch (t: Throwable) { // log safely, never rethrow } proceed() } scope.receivePipeline.intercept(HttpReceivePipeline.After) { withContext(config.context) { try { config.responseProcessor(subject) } catch (t: Throwable) { /* log */ } } proceed() } }src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt (4)
7-17: Missing import for Logging.sanitizeHeader usage (if you adopt the above hardening).Add Logger import if using the suggested
loggerconfig:import io.ktor.client.plugins.logging.LogLevel import io.ktor.client.plugins.logging.Logging +import io.ktor.client.plugins.logging.Logger
19-21: Prefer suspend main over runBlocking for CLI samples.Modern Kotlin supports
suspend fun main(); this avoids blocking the main thread explicitly and removes the need for suppression.-@Suppress("MagicNumber", "TooGenericExceptionCaught") -fun main() = runBlocking { +@Suppress("MagicNumber", "TooGenericExceptionCaught") +suspend fun main() {
66-67: Nit: grammar in comment."a endpoint" -> "an endpoint".
-// Test with a endpoint that might fail (for retry testing) +// Test with an endpoint that might fail (for retry testing)
23-33: Avoid double logging (built-in Logging + custom HttpLogInterceptor).Both plugins log requests/responses; this will duplicate output and add overhead. Consider disabling one or scoping each to different concerns (e.g., built-in Logging for HEADERS; custom interceptor for timing only).
src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt (1)
45-56: Consider explicitly continuing the response pipeline.While logging-only interceptors usually can return without
proceed(), addingproceed()makes the intent explicit and prevents accidental short-circuiting if future edits alter the subject.scope.responsePipeline.intercept(HttpResponsePipeline.Receive) { val startTime = context.request.attributes.getOrNull(RequestStartTimeKey) val durationMs = startTime?.let { (System.nanoTime() - it) / 1_000_000 } ?: 0 println("📥 RESPONSE: ${context.response.status} (${durationMs}ms)") if (plugin.logHeaders) { /* header printing */ } + proceed() }src/test/kotlin/dev/shtanko/api/ktor/KtorInterceptorsTest.kt (4)
36-77: Header expectation assumes default header name.The assertion checks for "X-API-Key". This couples the test to the interceptor’s default headerName. If the default changes (or in examples you use Authorization), this test will fail.
- Either set
headerNameexplicitly in the client under test, or- Make the test accept both header names.
Apply this diff to make the intent explicit:
install(ApiKeyInterceptor) { - apiKey = "integration-test-key" + apiKey = "integration-test-key" + headerName = "X-API-Key" }
82-118: Performance test relies on wall-clock; reduce flakiness.
@Timeout(5)uses real time. Even with MockEngine, CI hiccups can cause intermittent failures. Prefer measuring coroutine-time or asserting no excessive overhead by counting operations.Options:
- Use
kotlin.time.measureTimeand assert an upper bound less tight (e.g., < 15s), or- Parallelize requests and
advanceUntilIdle()withrunTestto avoid real delays.No diff provided since this is a non-functional test-flakiness concern.
147-166: Use a thread-safe collection or a mutex instead of synchronized on a mutable list.While
synchronized(processedRequests)works, aMutexor a thread-safe collection (e.g.,ConcurrentLinkedQueue) is clearer in coroutine tests.Example using a
Mutex:- val processedRequests = mutableListOf<String>() + val processedRequests = mutableListOf<String>() + val lock = kotlinx.coroutines.sync.Mutex() ... - requestProcessor = { request -> - synchronized(processedRequests) { - processedRequests.add(request.url.toString()) - } - } + requestProcessor = { request -> + lock.lock() + try { + processedRequests.add(request.url.toString()) + } finally { + lock.unlock() + } + }Also applies to: 168-183
185-219: Retry behavior is good; consider asserting retry delays via virtual time.Given
runTestand smallbaseDelayMs, you can assert that virtual time advanced as expected (if the RetryInterceptor usesdelay). This would strengthen the test without adding flakiness.Follow-up available if you expose the delay strategy or inject a test scheduler into the interceptor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (10)
README.md(1 hunks)api/Kotlin-Lab.api(4 hunks)build.gradle.kts(2 hunks)gradle/libs.versions.toml(2 hunks)src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt(1 hunks)src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt(1 hunks)src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt(1 hunks)src/main/kotlin/dev/shtanko/api/ktor/RequestResponseProcessorInterceptor.kt(1 hunks)src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt(1 hunks)src/test/kotlin/dev/shtanko/api/ktor/KtorInterceptorsTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- build.gradle.kts
- README.md
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt (4)
src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt (1)
install(27-32)src/main/kotlin/dev/shtanko/api/ktor/RequestResponseProcessorInterceptor.kt (1)
install(29-41)src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt (1)
install(31-57)src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt (1)
install(37-64)
src/test/kotlin/dev/shtanko/api/ktor/KtorInterceptorsTest.kt (5)
src/test/kotlin/dev/shtanko/concurrency/TestBase.kt (1)
runTest(67-103)src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt (1)
install(27-32)src/main/kotlin/dev/shtanko/api/ktor/RequestResponseProcessorInterceptor.kt (1)
install(29-41)src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt (1)
install(31-57)src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt (1)
install(37-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
gradle/libs.versions.toml (1)
82-84: JOL addition is fineCoordinates and version look correct, and the comment improves readability.
api/Kotlin-Lab.api (1)
18329-18333: KtorGitHubApi suspend API looks goodThe coroutine-based surface (suspend via Continuation) for getOrgRepos/getRepoContributors is idiomatic and aligns with Ktor usage. Good addition.
| public final class dev/shtanko/api/KtorGithubServiceKt { | ||
| public static final fun createKtorClient (Ljava/lang/String;Ljava/lang/String;Lio/ktor/client/engine/HttpClientEngine;)Lio/ktor/client/HttpClient; | ||
| public static synthetic fun createKtorClient$default (Ljava/lang/String;Ljava/lang/String;Lio/ktor/client/engine/HttpClientEngine;ILjava/lang/Object;)Lio/ktor/client/HttpClient; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
createKtorClient(String, String, HttpClientEngine) has ambiguous parameters; prefer a typed config/builder
Two adjacent String parameters are error-prone (hard to infer meaning at call sites). Introduce a config type or DSL builder and deprecate the current overload. This also centralizes defaults (engine, JSON, headers, plugins).
Example refactor (additions/changes in KtorGithubService.kt):
+data class KtorClientConfig(
+ val baseUrl: String,
+ val apiKey: String? = null,
+ val headerName: String = "Authorization",
+ val engine: HttpClientEngine? = null,
+)
+
+@JvmOverloads
+fun createKtorClient(
+ config: KtorClientConfig,
+ configure: HttpClientConfig<*>.() -> Unit = {}
+): HttpClient {
+ val client = HttpClient(config.engine ?: CIO) {
+ // install JSON, timeouts, etc.
+ if (!config.apiKey.isNullOrBlank()) {
+ install(ApiKeyInterceptor) {
+ apiKey = config.apiKey!!
+ headerName = config.headerName
+ }
+ }
+ configure()
+ }
+ return client
+}
+
+@Deprecated(
+ message = "Use createKtorClient(KtorClientConfig, ...) for clarity and safety.",
+ replaceWith = ReplaceWith("createKtorClient(KtorClientConfig(baseUrl, apiKey, headerName, engine))")
+)
-fun createKtorClient(baseUrl: String, apiKey: String, engine: HttpClientEngine): HttpClient
+fun createKtorClient(baseUrl: String, apiKey: String, engine: HttpClientEngine): HttpClientCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/Kotlin-Lab.api around lines 18335 to 18338, the createKtorClient(String,
String, HttpClientEngine) API uses two adjacent String parameters which are
ambiguous; replace this with a typed configuration (data class) or DSL builder
that encapsulates named fields (e.g., baseUrl, token, engine, jsonConfig,
headers, plugins), add a new overload createKtorClient(config: KtorClientConfig)
or builder-style createKtorClient { ... } that centralizes defaults, mark the
old String-based overload as @Deprecated with a migration message, update all
internal callsites to construct the new config or use the builder, and add a
small migration note in the KtorGithubService.kt public API indicating the
deprecation and showing how to migrate to the new config/builder form.
💡 Verification agent
🧩 Analysis chain
Automated verification: secrets redaction and retry policy
Quick repository scan to validate redaction and retry behavior.
Also applies to: 18470-18491, 18514-18535
🏁 Script executed:
#!/bin/bash
set -e
echo "1) Check for Authorization/API key values in logs/tests (should be redacted)..."
rg -n -i 'Authorization:\s+(?!\*\*\*|██REDACTED██)|X-Api-Key:\s+(?!\*\*\*|██REDACTED██)|Api-?Key:\s+(?!\*\*\*|██REDACTED██)' src/test || true
echo "2) Check HttpLogInterceptor for redaction config..."
rg -n -A2 -B2 -i 'redact|redactedHeaders|redactBody' src/main || true
echo "3) Check RetryInterceptor for jitter/backoff/idempotent-only..."
rg -n -A2 -B2 -i 'jitter|backoff|idempotent|maxDelay|Retry-After|429' src/main || trueLength of output: 44311
Action: Fix retry behavior (add jitter, cap, Retry-After/idempotency) and confirm header redaction
Scan summary (concise):
- Http logging already redacts Authorization: src/main/kotlin/dev/shtanko/github/data/network/di/di.kt — HttpLoggingInterceptor { level = BODY; redactHeader("Authorization") } (line ~41).
- The automated test-scan was inconclusive: the grep used a PCRE look‑around (ripgrep error). Re-run with --pcre2 or simpler patterns to confirm no API keys are hard-coded in tests.
- Retry logic gaps found: the Ktor interceptor uses plain exponential backoff and println logging; no jitter, no explicit max-delay/jitter cap, no Retry-After handling, and no idempotency restriction.
Files/locations needing attention:
- src/main/kotlin/dev/shtanko/github/data/network/di/di.kt — ensure all sensitive headers used by the API (e.g. "X-Api-Key", "Api-Key") are also redacted (add redactHeader(...) / redactBody if needed).
- src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt (≈ lines 48–52) — current code uses plugin.baseDelayMs * (1 shl attempt) and println(...); lacks jitter, Retry-After handling, and idempotent-only control.
- src/main/kotlin/dev/shtanko/retry/Retry.kt (≈ lines 71–99) — contains exponential/backoff helpers (maxDelay/factor). Consider reusing/aligning RetryInterceptor with this module (or add jitter here).
Recommended changes (short):
- Implement jitter (e.g., full or capped jitter) when computing delay and coerce to maxDelay.
- Honor Retry-After header and 429 responses; default to retry only idempotent methods (configurable).
- Replace println with proper logging (logger).
- Add redactHeader(...) for any API-key headers and add unit/integration tests asserting sensitive headers are redacted in logs.
- Re-run the secrets scan using a ripgrep command that supports look-arounds or a simpler pattern, e.g.:
rg --pcre2 -n -i 'Authorization:|X-Api-Key:|Api-?Key:' src/test
Tag:
🤖 Prompt for AI Agents
In api/Kotlin-Lab.api around lines 18335–18338 and affecting
src/main/kotlin/dev/shtanko/github/data/network/di/di.kt (~line 41),
src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt (≈48–52) and
src/main/kotlin/dev/shtanko/retry/Retry.kt (≈71–99): the retry implementation
lacks jitter, a max cap, Retry-After handling and idempotency checks and some
sensitive headers aren’t redacted; update di.kt to redact all API key header
names you use (e.g. "X-Api-Key","Api-Key","ApiKey") and redact body if needed;
refactor RetryInterceptor to compute delay = min(maxDelay, jitter(coordinated
backoff)) where jitter uses capped/full jitter, parse and honor Retry-After (and
treat 429 specially), only retry for idempotent HTTP methods by default
(configurable), and replace println calls with the project logger; align or
reuse helpers in Retry.kt (move jitter/maxDelay logic there or call it) and add
unit tests to assert header redaction in logs and correct retry behavior; after
changes re-run the secret scan with ripgrep using a compatible pattern e.g. rg
--pcre2 -n -i 'Authorization:|X-Api-Key:|Api-?Key:' src/test to confirm no
hard-coded keys remain.
| public static final fun logRepos (Ldev/shtanko/api/model/RequestData;Lretrofit2/Response;)V | ||
| public static final fun logUsers (Ldev/shtanko/api/model/Repo;Lretrofit2/Response;)V |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Logger: verify PII redaction for RequestData and auth headers
The logger now receives dev.shtanko.api.model types. Ensure it never logs RequestData.password or Authorization-like headers. If not already implemented, mask sensitive values at the formatter layer.
Run to verify no sensitive values are logged:
🏁 Script executed:
#!/bin/bash
# Expected: No occurrences of 'password' values or raw Authorization/API keys being logged.
echo "Searching for potential secret logging in logger and interceptors..."
rg -n -A2 -B2 'log(Repos|Users)\s*\('
rg -n -A2 -B2 -i 'authorization|x-api-key|api-?key|bearer|basic' src/main | sed 's/^/HINT: /'
rg -n -A2 -B2 -i 'password' src/main | sed 's/^/HINT: /'Length of output: 174127
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Logger.kt ==="
sed -n '1,240p' src/main/kotlin/dev/shtanko/api/contributors/Logger.kt || true
echo
echo "=== ApiKeyInterceptor.kt ==="
sed -n '1,240p' src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt || true
echo
echo "=== KtorGithubService.kt ==="
sed -n '1,240p' src/main/kotlin/dev/shtanko/api/KtorGithubService.kt || true
echo
echo "=== GitHubService.kt (createGitHubService snippet) ==="
sed -n '1,240p' src/main/kotlin/dev/shtanko/api/GitHubService.kt || true
echo
echo "=== okhttp/di.kt (HttpLoggingInterceptor) ==="
sed -n '1,240p' src/main/kotlin/dev/shtanko/github/data/network/di/di.kt || true
echo
echo "=== Search for raw logging of RequestData.password or Authorization header in repo ==="
rg -n --hidden -S --no-ignore-vcs -i "password|Authorization|HttpHeaders.Authorization|Authorization\"" src/main || true
echo
echo "=== Search for usages of logRepos/logUsers implementations ==="
rg -n --hidden -S --no-ignore-vcs "fun logRepos|fun logUsers|logRepos\(|logUsers\(" || trueLength of output: 24241
Logger: PII redaction verified — fixes needed for interceptors/logging
Short summary: logRepos/logUsers do NOT log RequestData.password. However I found places that can leak secrets or are not configured to redact Authorization/API keys.
Files to review/fix:
- src/main/kotlin/dev/shtanko/api/contributors/Logger.kt — OK: logs only req.org, repo.name, counts and response codes.
- src/main/kotlin/dev/shtanko/github/data/network/di/di.kt — OK: HttpLoggingInterceptor.redactHeader("Authorization") is set.
- src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt — issue: uses println("🔑 Added API key header: ${plugin.headerName}") and appends the API key header. Action: remove stdout printing, do not log the key; use a logger at debug level (without the secret) or drop the message entirely.
- src/main/kotlin/dev/shtanko/api/KtorGithubService.kt — issue: installs Ktor Logging (level = INFO) but no redaction configured. Action: configure the Ktor Logging plugin to avoid emitting Authorization/API-key headers or disable header/body logging; or implement header redaction in the logging layer.
- src/main/kotlin/dev/shtanko/api/GitHubService.kt (createHttpClient) — note: auth token is set on requests. If you add an OkHttp logging interceptor to this client in the future, ensure redactHeader("Authorization") is applied.
- src/main/kotlin/dev/shtanko/api/model/RequestData.kt and src/main/kotlin/dev/shtanko/api/contributors/Params.kt — RequestData.password is stored/used (and persisted to Preferences). Ensure any future logging of RequestData is sanitized (mask password) and consider secure storage for persisted passwords.
Recommended minimal changes:
- Remove or replace println in ApiKeyInterceptor.kt; never print or log the API key or full Authorization header.
- Configure Ktor Logging to exclude or redact Authorization (or set a safer LogLevel).
- Add a small sanitizer helper used by loggers/formatters that masks RequestData.password (e.g., show only username/org; mask password with ****) and use it wherever RequestData might be logged.
Tag:
🤖 Prompt for AI Agents
In api/Kotlin-Lab.api around lines 18402-18403, address PII leakage by: (1) Edit
src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt — remove the println
that prints the API key, replace with either no message or a logger.debug call
that never includes the header value (only log that a header was added, not its
content). (2) Edit src/main/kotlin/dev/shtanko/api/KtorGithubService.kt —
configure the Ktor Logging plugin to avoid emitting Authorization/API-Key
headers (either disable header/body logging or implement header redaction so
Authorization is omitted/masked), and set a safer log level. (3) Add a small
sanitizer helper (e.g., utils/sanitizers) that masks RequestData.password
(showing only non-sensitive fields and replacing password with ****) and use it
in Logger.kt and any place RequestData might be logged or persisted. Ensure
future OkHttp interceptors apply redactHeader("Authorization") when added.
| public final class dev/shtanko/api/ktor/ApiKeyInterceptor { | ||
| public static final field Companion Ldev/shtanko/api/ktor/ApiKeyInterceptor$Companion; | ||
| public synthetic fun <init> (Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/internal/DefaultConstructorMarker;)V | ||
| } | ||
|
|
||
| public final class dev/shtanko/api/ktor/ApiKeyInterceptor$Companion : io/ktor/client/plugins/HttpClientPlugin { | ||
| public fun getKey ()Lio/ktor/util/AttributeKey; | ||
| public fun install (Ldev/shtanko/api/ktor/ApiKeyInterceptor;Lio/ktor/client/HttpClient;)V | ||
| public synthetic fun install (Ljava/lang/Object;Lio/ktor/client/HttpClient;)V | ||
| public fun prepare (Lkotlin/jvm/functions/Function1;)Ldev/shtanko/api/ktor/ApiKeyInterceptor; | ||
| public synthetic fun prepare (Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; | ||
| } | ||
|
|
||
| public final class dev/shtanko/api/ktor/ApiKeyInterceptor$Config { | ||
| public fun <init> ()V | ||
| public final fun getApiKey ()Ljava/lang/String; | ||
| public final fun getHeaderName ()Ljava/lang/String; | ||
| public final fun setApiKey (Ljava/lang/String;)V | ||
| public final fun setHeaderName (Ljava/lang/String;)V | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ApiKeyInterceptor: scope to allowed hosts and prevent key leakage
Strengthen the plugin to avoid leaking secrets and to only attach the header when targeting trusted hosts/domains. Also allow a prefix (e.g., Bearer, token) to avoid hardcoding.
Suggested changes (ApiKeyInterceptor.kt):
class ApiKeyInterceptor {
companion object : HttpClientPlugin<ApiKeyInterceptor, ApiKeyInterceptor> {
override val key: AttributeKey<ApiKeyInterceptor> = AttributeKey("ApiKeyInterceptor")
override fun prepare(block: ApiKeyInterceptor.() -> Unit): ApiKeyInterceptor = ApiKeyInterceptor().apply(block)
override fun install(plugin: ApiKeyInterceptor, scope: HttpClient) {
- scope.sendPipeline.intercept(HttpSendPipeline.State) {
- context.header(plugin.headerName, plugin.apiKey)
- proceed()
- }
+ scope.sendPipeline.intercept(HttpSendPipeline.State) {
+ // Only add header for allowed hosts
+ val requestHost = context.url.host
+ if (plugin.allowedHosts.isEmpty() || plugin.allowedHosts.any { requestHost.endsWith(it) }) {
+ val value = if (plugin.prefix.isNullOrBlank()) plugin.apiKey else "${plugin.prefix} ${plugin.apiKey}"
+ context.headers.append(plugin.headerName, value, safeOnly = true)
+ }
+ proceed()
+ }
}
}
class Config {
var apiKey: String = ""
var headerName: String = "Authorization"
+ var prefix: String? = "Bearer" // or "token" for GitHub
+ var allowedHosts: Set<String> = setOf("api.github.com")
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/Kotlin-Lab.api around lines 18449 to 18468, the ApiKeyInterceptor
currently always attaches the API key and risks leaking secrets and adding
headers to untrusted hosts; update the plugin so Config exposes apiKey,
headerName and optional prefix (e.g., "Bearer "), and expose an
allowedHosts/allowedDomains list; in install, add a request pipeline interceptor
that checks the request URL host against allowedHosts and only then adds the
header value as prefix + apiKey (avoid any logging or toString of the raw key),
skip adding the header for requests with no host, and ensure prepare/Config set
sensible defaults while keeping the key private (do not expose it via public
logs or attribute keys).
| public final class dev/shtanko/api/ktor/HttpLogInterceptor { | ||
| public static final field Companion Ldev/shtanko/api/ktor/HttpLogInterceptor$Companion; | ||
| public synthetic fun <init> (ZZZLkotlin/jvm/internal/DefaultConstructorMarker;)V | ||
| } | ||
|
|
||
| public final class dev/shtanko/api/ktor/HttpLogInterceptor$Companion : io/ktor/client/plugins/HttpClientPlugin { | ||
| public fun getKey ()Lio/ktor/util/AttributeKey; | ||
| public fun install (Ldev/shtanko/api/ktor/HttpLogInterceptor;Lio/ktor/client/HttpClient;)V | ||
| public synthetic fun install (Ljava/lang/Object;Lio/ktor/client/HttpClient;)V | ||
| public fun prepare (Lkotlin/jvm/functions/Function1;)Ldev/shtanko/api/ktor/HttpLogInterceptor; | ||
| public synthetic fun prepare (Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; | ||
| } | ||
|
|
||
| public final class dev/shtanko/api/ktor/HttpLogInterceptor$Config { | ||
| public fun <init> ()V | ||
| public final fun getLogHeaders ()Z | ||
| public final fun getLogRequest ()Z | ||
| public final fun getLogResponse ()Z | ||
| public final fun setLogHeaders (Z)V | ||
| public final fun setLogRequest (Z)V | ||
| public final fun setLogResponse (Z)V | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
HttpLogInterceptor: redact sensitive headers and bodies by default
Ensure the logging plugin never prints secrets by default (Authorization, X-Api-Key, Cookie, Set-Cookie) and optionally body content. Provide a redaction allowlist/denylist in config.
Proposed config and usage (HttpLogInterceptor.kt):
class HttpLogInterceptor {
companion object : HttpClientPlugin<HttpLogInterceptor, HttpLogInterceptor> { ... }
class Config {
var logRequest: Boolean = true
var logResponse: Boolean = true
var logHeaders: Boolean = false
+ var redactedHeaders: Set<String> =
+ setOf("Authorization", "Proxy-Authorization", "X-Api-Key", "X-API-KEY", "Api-Key", "Cookie", "Set-Cookie")
+ var redactBody: Boolean = true
}
}
-// When logging headers:
+// When logging headers:
+headers.forEach { (name, value) ->
+ val safe = if (config.redactedHeaders.any { it.equals(name, ignoreCase = true) }) "██REDACTED██" else value
+ log("$name: $safe")
+}To check for potential leaks in tests/logs:
🏁 Script executed:
#!/bin/bash
# Expected: Logs should not contain raw Authorization values or tokens.
rg -n -i 'Authorization:\s*(?!\*\*\*|██REDACTED██)' src/test || true
rg -n -i 'Bearer\s+[A-Za-z0-9_\-]+' src/test || trueLength of output: 384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for HttpLogInterceptor and related identifiers..."
rg -n --hidden -S "HttpLogInterceptor" -g '!*node_modules/*' || true
rg -n --hidden -S "class HttpLogInterceptor" -g '!*node_modules/*' || true
rg -n --hidden -S "HttpLogInterceptor.kt" -g '!*node_modules/*' || true
echo
echo "Searching for existing config/logging flags and redaction-related identifiers..."
rg -n --hidden -S "logHeaders|logRequest|logResponse|redact|redacted|redactBody|redactedHeaders" -g '!*node_modules/*' || true
echo
echo "Show surrounding context for matches (if any) for the top 10 matches for HttpLogInterceptor..."
rg -n --hidden -S "HttpLogInterceptor" -g '!*node_modules/*' -C 3 | sed -n '1,200p' || trueLength of output: 12118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== HttpLogInterceptor.kt (first 240 lines) ==="
sed -n '1,240p' src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt || true
echo
echo "=== Search for redactHeader occurrences ==="
rg -n --hidden -S "redactHeader" -g '!*node_modules/*' || true
echo
echo "=== Show di.kt around redactHeader usage ==="
sed -n '1,240p' src/main/kotlin/dev/shtanko/github/data/network/di/di.kt || true
echo
echo "=== Search for 'REDACTED' or '██REDACTED██' in repo ==="
rg -n --hidden -S "REDACTED|██REDACTED██" -g '!*node_modules/*' || trueLength of output: 6759
HttpLogInterceptor: redact sensitive headers and bodies by default
The Ktor logging plugin currently prints headers verbatim when logHeaders = true (see install() in HttpLogInterceptor). That will leak secrets (Authorization, Cookie, API keys). OkHttp logging already uses redactHeader("Authorization") in di.kt — align the Ktor plugin with that behavior.
Files to change
- src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt
- Add redaction configuration to Config.
- Extend HttpLogInterceptor constructor to accept redactedHeaders and redactBody.
- Replace direct header prints with per-header case-insensitive redaction when logging requests/responses.
- (Optional) Update tests if they assert exact header output.
Suggested diff (concise)
class HttpLogInterceptor private constructor(
- private val logRequest: Boolean,
- private val logResponse: Boolean,
- private val logHeaders: Boolean,
+ private val logRequest: Boolean,
+ private val logResponse: Boolean,
+ private val logHeaders: Boolean,
+ private val redactedHeaders: Set<String>,
+ private val redactBody: Boolean,
) {
class Config {
var logRequest: Boolean = true
var logResponse: Boolean = true
var logHeaders: Boolean = false
+ var redactedHeaders: Set<String> = setOf(
+ "Authorization", "Proxy-Authorization", "X-Api-Key", "X-API-KEY", "Api-Key", "Cookie", "Set-Cookie"
+ )
+ var redactBody: Boolean = true
}
companion object : HttpClientPlugin<Config, HttpLogInterceptor> {
@@
- return HttpLogInterceptor(config.logRequest, config.logResponse, config.logHeaders)
+ return HttpLogInterceptor(
+ config.logRequest,
+ config.logResponse,
+ config.logHeaders,
+ config.redactedHeaders,
+ config.redactBody
+ )
@@
- if (plugin.logHeaders) {
- println(" Headers: ${context.headers.entries()}")
- }
+ if (plugin.logHeaders) {
+ context.headers.entries().forEach { (name, values) ->
+ val safe = if (plugin.redactedHeaders.any { it.equals(name, ignoreCase = true) })
+ listOf("██REDACTED██") else values
+ println(" $name: $safe")
+ }
+ }
@@
- if (plugin.logHeaders) {
- println(" Headers: ${context.response.headers.entries()}")
- }
+ if (plugin.logHeaders) {
+ context.response.headers.entries().forEach { (name, values) ->
+ val safe = if (plugin.redactedHeaders.any { it.equals(name, ignoreCase = true) })
+ listOf("██REDACTED██") else values
+ println(" $name: $safe")
+ }
+ }Quick verification notes
- I inspected src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt — it currently prints headers via context.headers.entries() and context.response.headers.entries().
- OkHttp provider (src/main/kotlin/dev/shtanko/github/data/network/di/di.kt) already calls HttpLoggingInterceptor().redactHeader("Authorization").
Suggested quick checks (run locally)
- Search for raw Authorization or bearer tokens in repo/logs:
- rg --pcre2 -n -i 'Authorization:\s*(?!***|██REDACTED██).+' src || true
- rg -n -i 'Bearer\s+[A-Za-z0-9_-]+' src || true
Reasoning: make redaction default (or at least default denylist) so enabling logHeaders cannot leak secrets. Add redactBody for future body-logging support; keep logHeaders default false as now.
🤖 Prompt for AI Agents
In src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt around the
install()/Config code (lines ~18470-18491), the plugin currently prints
request/response headers verbatim; add a redaction mechanism by extending Config
with a redactedHeaders Set<String> (defaulting to at least "Authorization" and
"Cookie", case-insensitive) and a redactBody Boolean (default false), extend the
HttpLogInterceptor constructor to accept these redaction settings, and change
header logging to check each header name case-insensitively against the
redactedHeaders set and replace the value with a fixed placeholder (e.g.
"***REDACTED***") before printing; keep existing logHeaders default behavior but
ensure redaction is applied whenever logHeaders=true so secrets aren’t leaked
(update constructor/prepare/install to pass config.redactedHeaders and
config.redactBody through).
| public final class dev/shtanko/api/ktor/RetryInterceptor { | ||
| public static final field Companion Ldev/shtanko/api/ktor/RetryInterceptor$Companion; | ||
| public synthetic fun <init> (ILjava/util/Set;JLkotlin/jvm/internal/DefaultConstructorMarker;)V | ||
| } | ||
|
|
||
| public final class dev/shtanko/api/ktor/RetryInterceptor$Companion : io/ktor/client/plugins/HttpClientPlugin { | ||
| public fun getKey ()Lio/ktor/util/AttributeKey; | ||
| public fun install (Ldev/shtanko/api/ktor/RetryInterceptor;Lio/ktor/client/HttpClient;)V | ||
| public synthetic fun install (Ljava/lang/Object;Lio/ktor/client/HttpClient;)V | ||
| public fun prepare (Lkotlin/jvm/functions/Function1;)Ldev/shtanko/api/ktor/RetryInterceptor; | ||
| public synthetic fun prepare (Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; | ||
| } | ||
|
|
||
| public final class dev/shtanko/api/ktor/RetryInterceptor$Config { | ||
| public fun <init> ()V | ||
| public final fun getBaseDelayMs ()J | ||
| public final fun getMaxRetries ()I | ||
| public final fun getRetryOn ()Ljava/util/Set; | ||
| public final fun setBaseDelayMs (J)V | ||
| public final fun setMaxRetries (I)V | ||
| public final fun setRetryOn (Ljava/util/Set;)V | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Retry policy: enforce idempotency, exponential backoff + jitter, and cap delays
Retries should default to idempotent methods only (GET/HEAD/OPTIONS) and include exponential backoff with jitter, handling 429 Retry-After.
Proposed changes (RetryInterceptor.kt):
class RetryInterceptor {
class Config {
- var maxRetries: Int = 3
- var baseDelayMs: Long = 250
- var retryOn: Set<Int> = setOf(429, 502, 503, 504)
+ var maxRetries: Int = 3
+ var baseDelayMs: Long = 250
+ var maxDelayMs: Long = 5_000
+ var jitterFactor: Double = 0.5
+ var retryOn: Set<Int> = setOf(429, 502, 503, 504)
+ var idempotentOnly: Boolean = true
}
companion object : HttpClientPlugin<Config, RetryInterceptor> {
override fun install(plugin: RetryInterceptor, scope: HttpClient) {
scope.receivePipeline.intercept(HttpReceivePipeline.After) {
- // naive loop
+ // exponential backoff with jitter, respect Retry-After
+ suspend fun backoff(attempt: Int): Long {
+ val base = min(plugin.baseDelayMs * (1 shl (attempt - 1)), plugin.maxDelayMs)
+ val jitter = (base * plugin.jitterFactor).toLong()
+ return (base - jitter..base + jitter).random()
+ }
+ // Only retry idempotent methods unless overridden
+ if (plugin.idempotentOnly && !setOf(HttpMethod.Get, HttpMethod.Head, HttpMethod.Options).contains(context.method)) {
+ return@intercept
+ }
}
}
}
}Verification helper:
🏁 Script executed:
#!/bin/bash
# Expected: presence of jitter/backoff, idempotent-only logic, and handling of 429/Retry-After.
rg -n -A2 -B2 -i 'Retry-After|jitter|maxDelay|idempotent' src/main || trueLength of output: 2215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg --hidden -n 'src/main/kotlin/dev/shtanko/retry/Retry.kt' || true
echo "----"
sed -n '1,240p' src/main/kotlin/dev/shtanko/retry/Retry.kt || trueLength of output: 4202
Action required — add jitter + Retry-After handling to the repo retry utility (original Ktor RetryInterceptor not present)
Short: I inspected src/main/kotlin/dev/shtanko/retry/Retry.kt. It already implements exponential backoff but lacks jitter and does not honor 429 Retry-After. The original comment referenced a Ktor RetryInterceptor that isn't in this repo — apply the fixes to the retry utility below (or to your HTTP client plugin if/when you add one).
Files to change
- src/main/kotlin/dev/shtanko/retry/Retry.kt — RetryConstants, DefaultRetryPolicy, and retry(...) loop.
Suggested minimal changes (diff-style):
object RetryConstants {
const val DEFAULT_INITIAL_DELAY: Long = 1000L
const val DEFAULT_MAX_DELAY: Long = 10_000L
const val DEFAULT_FACTOR: Double = 2.0
+ /** Fraction of the base delay to use as jitter (0.0..1.0) */
+ const val DEFAULT_JITTER_FACTOR: Double = 0.5
}
-class DefaultRetryPolicy(
- private val maxAttempts: Int = 3,
- private val retryIf: (Throwable) -> Boolean = { it is IOException || it is HttpException },
-) : RetryPolicy {
-
- override fun shouldRetry(throwable: Throwable, attempt: Int): Boolean {
- return attempt < maxAttempts && retryIf(throwable)
- }
-}
+class DefaultRetryPolicy(
+ private val maxAttempts: Int = 3,
+ private val retryStatusCodes: Set<Int> = setOf(429, 502, 503, 504),
+) : RetryPolicy {
+ override fun shouldRetry(throwable: Throwable, attempt: Int): Boolean {
+ if (attempt >= maxAttempts) return false
+ if (throwable is IOException) return true
+ if (throwable is HttpException) {
+ // only retry specific HTTP status codes
+ return retryStatusCodes.contains(throwable.code())
+ }
+ return false
+ }
+}
@Suppress("TooGenericExceptionCaught")
suspend fun <T> retry(
initialDelay: Long = RetryConstants.DEFAULT_INITIAL_DELAY,
maxDelay: Long = RetryConstants.DEFAULT_MAX_DELAY,
factor: Double = RetryConstants.DEFAULT_FACTOR,
+ jitterFactor: Double = RetryConstants.DEFAULT_JITTER_FACTOR,
policy: RetryPolicy = DefaultRetryPolicy(),
block: suspend (Int) -> T,
): T {
val currentDelay = AtomicLong(initialDelay)
val attempt = AtomicInteger(0)
while (true) {
try {
return block(attempt.get())
} catch (e: Throwable) {
val currentAttempt = attempt.incrementAndGet()
if (!policy.shouldRetry(e, currentAttempt)) {
// If the policy indicates no retry, rethrow the exception
throw e
}
- delay(currentDelay.get())
- currentDelay.set((currentDelay.get() * factor).toLong().coerceAtMost(maxDelay))
+ // If server returned 429 and provided a numeric Retry-After header, honor it (seconds -> ms).
+ val retryAfterHeader = (e as? HttpException)?.response()?.headers()?.get("Retry-After")
+ val retryAfterMillis = retryAfterHeader?.toLongOrNull()?.times(1000L)
+
+ val baseDelay = currentDelay.get()
+ val delayMs = retryAfterMillis ?: run {
+ val jitter = (baseDelay * jitterFactor).toLong()
+ val min = (baseDelay - jitter).coerceAtLeast(0L)
+ val max = baseDelay + jitter
+ // random in [min, max]
+ kotlin.random.Random.nextLong(min, max + 1)
+ }
+
+ delay(delayMs)
+ currentDelay.set((baseDelay * factor).toLong().coerceAtMost(maxDelay))
}
}
}Notes / rationale
- The repo already had exponential backoff; this patch adds bounded jitter (to avoid thundering herd) and honors numeric Retry-After values for 429.
- Idempotency checks (retry only on GET/HEAD/OPTIONS) require HTTP method context — that is appropriate at the HTTP client plugin/interceptor layer, not inside this generic retry helper. If you do have a Ktor/OkHttp/Retrofit interceptor, enforce idempotent-only there.
- The DefaultRetryPolicy change makes retry decisions aware of status codes (429/502/503/504) instead of blindly retrying all HttpException instances; adjust the set if your API needs differ.
- This implementation only parses numeric Retry-After values (seconds). If you need to support HTTP-date formats, add a date parser and convert to millis.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/kotlin/dev/shtanko/retry/Retry.kt (around the definitions of
RetryConstants, DefaultRetryPolicy, and the retry(...) loop), update the retry
logic to add bounded jitter and honor numeric Retry-After for 429 responses:
change RetryConstants to include a jitterMaxMs and a maxDelayCap, update
DefaultRetryPolicy to only consider specific retryable status codes (at minimum
429, 502, 503, 504) instead of all HttpException instances, and expose a way for
the policy to accept an HTTP response/status and optional headers; in the
retry(...) loop, when computing backoff use exponential backoff multiplied by a
cap and then add a randomized jitter in the range [0, jitterMaxMs], and if the
response is 429 and a numeric Retry-After header (seconds) is present, use
Retry-After (converted to ms) as the delay for that attempt (still capped by
maxDelayCap and with jitter applied), finally ensure delays are awaited before
retries and keep idempotency checks out of this generic helper (enforce
method-level idempotency in your HTTP client plugin/interceptor).
| class Config { | ||
| var requestProcessor: (HttpRequestBuilder) -> Unit = { } | ||
| var responseProcessor: suspend (HttpResponse) -> Unit = { } | ||
| } |
There was a problem hiding this comment.
Default no-op lambdas won’t compile (missing parameters).
Both default lambdas must accept parameters to match their function types.
Apply this diff:
class Config {
- var requestProcessor: (HttpRequestBuilder) -> Unit = { }
- var responseProcessor: suspend (HttpResponse) -> Unit = { }
+ var requestProcessor: (HttpRequestBuilder) -> Unit = { _ -> }
+ var responseProcessor: suspend (HttpResponse) -> Unit = { _: HttpResponse -> }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class Config { | |
| var requestProcessor: (HttpRequestBuilder) -> Unit = { } | |
| var responseProcessor: suspend (HttpResponse) -> Unit = { } | |
| } | |
| class Config { | |
| var requestProcessor: (HttpRequestBuilder) -> Unit = { _ -> } | |
| var responseProcessor: suspend (HttpResponse) -> Unit = { _: HttpResponse -> } | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/dev/shtanko/api/ktor/RequestResponseProcessorInterceptor.kt
around lines 16 to 19, the default no-op lambdas don't declare the required
parameters and therefore won't compile; update the defaults so they accept the
expected parameters (for example set requestProcessor to a lambda that accepts
an HttpRequestBuilder parameter and responseProcessor to a suspend lambda that
accepts an HttpResponse parameter, e.g. use placeholders like { _:
HttpRequestBuilder -> } and { _: HttpResponse -> } respectively) so the default
values match their function types.
| override fun prepare(block: Config.() -> Unit): RetryInterceptor { | ||
| val config = Config().apply(block) | ||
| return RetryInterceptor(config.maxRetries, config.retryOn, config.baseDelayMs) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate config values in prepare to fail fast on misconfiguration.
Negative retries or delay will produce runtime errors.
Apply this diff:
override fun prepare(block: Config.() -> Unit): RetryInterceptor {
val config = Config().apply(block)
+ require(config.maxRetries >= 0) { "maxRetries must be >= 0" }
+ require(config.baseDelayMs >= 0) { "baseDelayMs must be >= 0" }
return RetryInterceptor(config.maxRetries, config.retryOn, config.baseDelayMs)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override fun prepare(block: Config.() -> Unit): RetryInterceptor { | |
| val config = Config().apply(block) | |
| return RetryInterceptor(config.maxRetries, config.retryOn, config.baseDelayMs) | |
| } | |
| override fun prepare(block: Config.() -> Unit): RetryInterceptor { | |
| val config = Config().apply(block) | |
| require(config.maxRetries >= 0) { "maxRetries must be >= 0" } | |
| require(config.baseDelayMs >= 0) { "baseDelayMs must be >= 0" } | |
| return RetryInterceptor(config.maxRetries, config.retryOn, config.baseDelayMs) | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt around lines 32 to
35, validate the Config values in prepare to fail fast: after creating val
config = Config().apply(block) check that config.maxRetries is >= 0 and
config.baseDelayMs is >= 0 (and optionally validate retryOn if your semantics
require a non-empty set), and if any validation fails throw an
IllegalArgumentException with a clear message indicating which property is
invalid and its value; only then return RetryInterceptor(config.maxRetries,
config.retryOn, config.baseDelayMs).
| install(Logging) { | ||
| level = LogLevel.BODY | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not log secrets and full bodies in examples; sanitize headers and avoid LogLevel.BODY.
With logHeaders = true and the built-in Logging at LogLevel.BODY, Authorization and other secrets get printed to stdout. This is a security risk and an antipattern in sample code.
- Disable header logging in your custom interceptor or
- Keep header logging but sanitize sensitive headers and reduce built-in logging to HEADERS.
Apply this diff to harden logging here:
install(Logging) {
- level = LogLevel.BODY
+ level = LogLevel.HEADERS
+ // Optional: if you keep built-in Logging, sanitize secrets
+ logger = io.ktor.client.plugins.logging.Logger.DEFAULT
+ sanitizeHeader { h ->
+ h.equals(HttpHeaders.Authorization, ignoreCase = true) ||
+ h.equals("X-API-Key", ignoreCase = true)
+ }
}
// Install custom interceptors
install(HttpLogInterceptor) {
logRequest = true
logResponse = true
- logHeaders = true
+ // Disable header logging here to avoid leaking secrets (or add redaction in the plugin)
+ logHeaders = false
}
install(ApiKeyInterceptor) {
- apiKey = "your-api-key-here"
- headerName = "Authorization"
+ // Prefer Authorization bearer token or a custom header; do not log it
+ headerName = HttpHeaders.Authorization
+ apiKey = "Bearer ${System.getenv("API_TOKEN") ?: "your-api-key-here"}"
}Also applies to: 28-37
🤖 Prompt for AI Agents
In src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt around lines 23-25
(and similarly lines 28-37), the example enables LogLevel.BODY which prints full
request/response bodies and headers (including Authorization/Cookie) — change
the logging to avoid leaking secrets: set the built-in Logging to at most
LogLevel.HEADERS (not BODY) or disable it entirely, and in your custom
interceptor disable header logging or explicitly sanitize sensitive headers
(Authorization, Cookie, Set-Cookie, X-Api-Key, etc.) before logging; ensure no
full bodies are logged by removing any body logging calls and only log
non-sensitive headers or summary information.
# Conflicts: # README.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… into feature/ktor_example
|
# Conflicts: # README.md # build.gradle.kts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt (3)
23-26: Avoid logging bodies and redact sensitive headers in built-in LoggingLogging at BODY prints full payloads and headers, risking secret leakage. Switch to HEADERS and redact sensitive headers.
- install(Logging) { - level = LogLevel.BODY - } + install(Logging) { + level = LogLevel.HEADERS + // Keep stdout logging minimal and safe + logger = io.ktor.client.plugins.logging.Logger.DEFAULT + sanitizeHeader { h -> + h.equals(HttpHeaders.Authorization, ignoreCase = true) || + h.equals(HttpHeaders.Cookie, ignoreCase = true) || + h.equals(HttpHeaders.SetCookie, ignoreCase = true) || + h.equals("X-API-Key", ignoreCase = true) + } + }
35-39: Do not print request/response headers unredactedYour custom interceptor prints headers including Authorization, Cookies, etc. Disable header logging here (or implement redaction).
install(HttpLogInterceptor) { logRequest = true logResponse = true - logHeaders = true + // Avoid leaking secrets in sample output + logHeaders = false }
41-44: Avoid hardcoding API secrets; use Authorization constant and env-sourced tokenHardcoding tokens is unsafe and “Authorization” should use the constant. Prefer a Bearer token from env and ensure it’s never logged.
install(ApiKeyInterceptor) { - apiKey = "your-api-key-here" - headerName = "Authorization" + headerName = HttpHeaders.Authorization + apiKey = "Bearer ${System.getenv("API_TOKEN") ?: "your-api-key-here"}" }api/Kotlin-Lab.api (6)
18402-18403: Logger must sanitize RequestData and response headers; never log passwords or auth headersThese API signatures pass RequestData and retrofit Response to logging. Ensure:
- RequestData.password is never logged (don’t rely on default toString).
- Mask Authorization/Cookie-like headers in responses.
Quick check:
#!/bin/bash set -euo pipefail # Ensure logger doesn't log RequestData directly or 'password' rg -n -C2 -i 'logRepos\(|logUsers\(' src/main/kotlin/dev/shtanko/api/contributors rg -n -C2 -i 'password' src/main/kotlin/dev/shtanko/api/contributors || true # Ensure no raw Authorization/API-Key in logger/interceptors rg -n -C2 -i 'Authorization|X-Api-Key|Api-?Key' src/main/kotlin/dev/shtanko/api/ktor src/main/kotlin/dev/shtanko/api/contributors | sed 's/^/HINT: /'
18335-18338: Replace ambiguous String parameters with a typed config/builder and deprecate this overloadTwo adjacent String parameters (
baseUrl: String, apiKey: String) are error-prone. Introduce a typedKtorClientConfig(or DSL builder) and deprecate this overload to improve clarity and future extensibility (headers, redaction, retry, allowed hosts).Apply in src/main/kotlin/dev/shtanko/api/KtorGithubService.kt:
+data class KtorClientConfig( + val baseUrl: String, + val apiKey: String? = null, + val headerName: String = "Authorization", + val engine: HttpClientEngine? = null, +) + +@JvmOverloads +fun createKtorClient( + config: KtorClientConfig, + configure: HttpClientConfig<*>.() -> Unit = {} +): HttpClient { + val engine = config.engine ?: CIO.create() + return HttpClient(engine) { + // install JSON, timeouts, logging, retry, etc. + if (!config.apiKey.isNullOrBlank()) { + install(ApiKeyInterceptor) { + apiKey = config.apiKey!! + headerName = config.headerName + } + } + configure() + } +} + +@Deprecated( + message = "Use createKtorClient(KtorClientConfig, ...) for clarity and safety.", + replaceWith = ReplaceWith("createKtorClient(KtorClientConfig(baseUrl, apiKey, headerName = \"Authorization\", engine))") +) fun createKtorClient( - baseUrl: String, - apiKey: String, - engine: HttpClientEngine + baseUrl: String, + apiKey: String, + engine: HttpClientEngine ): HttpClient { /* existing body */ }
18449-18468: ApiKeyInterceptor: scope to allowed hosts, support prefix, and avoid exposure/logging of secretsThe config currently exposes the raw apiKey and header name, and the plugin will attach the header to every request. Limit blast radius and remove any stdout logging of secrets:
- Add allowedHosts and optional prefix (e.g., "Bearer" or "token") to Config.
- Append header only for trusted hosts.
- Never log the key value; use a proper logger without sensitive data.
Patch src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt:
class ApiKeyInterceptor { companion object : HttpClientPlugin<ApiKeyInterceptor, ApiKeyInterceptor> { @@ - scope.sendPipeline.intercept(HttpSendPipeline.State) { - context.header(plugin.headerName, plugin.apiKey) - proceed() - } + scope.sendPipeline.intercept(HttpSendPipeline.State) { + val requestHost = context.url.host + if (plugin.allowedHosts.isEmpty() || plugin.allowedHosts.any { requestHost.endsWith(it) }) { + val value = if (plugin.prefix.isNullOrBlank()) plugin.apiKey else "${plugin.prefix} ${plugin.apiKey}" + context.headers.append(plugin.headerName, value, safeOnly = true) + } + proceed() + } } - class Config { - var apiKey: String = "" - var headerName: String = "Authorization" - } + class Config { + var apiKey: String = "" + var headerName: String = "Authorization" + var prefix: String? = "Bearer" // or "token" for GitHub + var allowedHosts: Set<String> = setOf("api.github.com") + } }
18470-18491: HttpLogInterceptor: redact sensitive headers and avoid println; add redactBody optionCurrent Config only controls booleans; when headers are logged, values will leak unless redacted. Introduce a redactedHeaders allowlist and optional body redaction, and use a logger instead of println.
Patch src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt:
-class HttpLogInterceptor private constructor( - private val logRequest: Boolean, - private val logResponse: Boolean, - private val logHeaders: Boolean, -) { +class HttpLogInterceptor private constructor( + private val logRequest: Boolean, + private val logResponse: Boolean, + private val logHeaders: Boolean, + private val redactedHeaders: Set<String>, + private val redactBody: Boolean, +) { class Config { var logRequest: Boolean = true var logResponse: Boolean = true var logHeaders: Boolean = false + var redactedHeaders: Set<String> = setOf( + "Authorization", "Proxy-Authorization", "X-Api-Key", "X-API-KEY", "Api-Key", "Cookie", "Set-Cookie" + ) + var redactBody: Boolean = true } companion object : HttpClientPlugin<Config, HttpLogInterceptor> { @@ - return HttpLogInterceptor(config.logRequest, config.logResponse, config.logHeaders) + return HttpLogInterceptor( + config.logRequest, + config.logResponse, + config.logHeaders, + config.redactedHeaders, + config.redactBody, + ) } @@ - if (plugin.logHeaders) { - println(" Headers: ${context.headers.entries()}") - } + if (plugin.logHeaders) { + context.headers.entries().forEach { (name, values) -> + val safe = if (plugin.redactedHeaders.any { it.equals(name, ignoreCase = true) }) + listOf("██REDACTED██") else values + log.debug(" {}: {}", name, safe) + } + } @@ - if (plugin.logHeaders) { - println(" Headers: ${context.response.headers.entries()}") - } + if (plugin.logHeaders) { + context.response.headers.entries().forEach { (name, values) -> + val safe = if (plugin.redactedHeaders.any { it.equals(name, ignoreCase = true) }) + listOf("██REDACTED██") else values + log.debug(" {}: {}", name, safe) + } + }
18514-18535: RetryInterceptor: add jitter, maxDelay cap, Retry-After support, and idempotent-only defaultCurrent API exposes baseDelayMs/maxRetries/retryOn only. Add:
- maxDelayMs and jitterFactor to avoid thundering herd.
- idempotentOnly flag (default true) to restrict retries to GET/HEAD/OPTIONS.
- Honor numeric Retry-After on 429.
Patch src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt:
class RetryInterceptor { class Config { var maxRetries: Int = 3 var baseDelayMs: Long = 250 - var retryOn: Set<Int> = setOf(429, 502, 503, 504) + var maxDelayMs: Long = 5_000 + var jitterFactor: Double = 0.5 + var retryOn: Set<Int> = setOf(429, 502, 503, 504) + var idempotentOnly: Boolean = true } companion object : HttpClientPlugin<Config, RetryInterceptor> { override fun install(plugin: RetryInterceptor, scope: HttpClient) { scope.receivePipeline.intercept(HttpReceivePipeline.After) { - // naive loop + // Retry policy: idempotent-only (opt-out), exponential backoff with jitter, Retry-After aware + suspend fun backoff(attempt: Int, response: io.ktor.client.statement.HttpResponse?): Long { + response?.headers?.get("Retry-After")?.toLongOrNull()?.let { return it * 1000L } + val base = (plugin.baseDelayMs * (1L shl (attempt - 1))).coerceAtMost(plugin.maxDelayMs) + val jitter = (base * plugin.jitterFactor).toLong() + val min = (base - jitter).coerceAtLeast(0L) + val max = base + jitter + return kotlin.random.Random.nextLong(min, max + 1) + } + if (plugin.idempotentOnly && context.request.method !in setOf(HttpMethod.Get, HttpMethod.Head, HttpMethod.Options)) { + return@intercept + } + // implement your retry loop around receive or using a feature-local state... } } } }If you also maintain src/main/kotlin/dev/shtanko/retry/Retry.kt, align its backoff with the same jitter and Retry-After behavior to keep policies consistent.
18566-18580: Critical: RequestData exposes getPassword() and default toString(), risking PII leaksThe public getter and default toString make it easy to leak a plain-text password via logs/serialization. Redact and avoid serialization of passwords; prefer token-based auth.
Patch src/main/kotlin/dev/shtanko/api/model/RequestData.kt:
-@kotlinx.serialization.Serializable -data class RequestData( - val org: String, - val username: String, - val password: String, -) +@kotlinx.serialization.Serializable +data class RequestData( + val org: String, + val username: String, + @kotlinx.serialization.Transient val password: String = "", +) { + override fun toString(): String = "RequestData(org=$org, username=$username, password=██REDACTED██)" +}Optionally migrate off raw passwords:
- Prefer Authorization header with a token (
ApiKeyInterceptorwith prefix "Bearer" or "token").- If you must hold credentials in memory, consider
CharArrayinstead ofStringand clear it after use (requires custom serializer or exclusion from serialization).
🧹 Nitpick comments (2)
src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt (2)
77-81: Preserve diagnostic detail on errorsPrinting only e.message loses stack traces. Log to stderr and include the stack trace for debugging.
- } catch (e: Exception) { - println("Error: ${e.message}") + } catch (e: Exception) { + System.err.println("Error: ${e.message}") + e.printStackTrace() } finally { client.close() }
20-21: Prefer specific exception handling over suppressing TooGenericExceptionCaughtCatching Exception broadly is an anti-pattern. Either remove the suppression and catch specific Ktor/IO exceptions, or justify the generic catch.
-@Suppress("MagicNumber", "TooGenericExceptionCaught") +@Suppress("MagicNumber")If you choose to be specific, consider:
- io.ktor.client.plugins.ClientRequestException
- io.ktor.client.plugins.ServerResponseException
- java.io.IOException
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md(1 hunks)api/Kotlin-Lab.api(4 hunks)build.gradle.kts(2 hunks)gradle/libs.versions.toml(2 hunks)src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- build.gradle.kts
- gradle/libs.versions.toml
- README.md
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt (4)
src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt (1)
install(37-64)src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt (1)
install(27-32)src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt (1)
install(31-57)src/main/kotlin/dev/shtanko/api/ktor/RequestResponseProcessorInterceptor.kt (1)
install(29-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build
🔇 Additional comments (5)
src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt (1)
28-33: Good addition: HTTP timeouts prevent hangsTimeouts are set to reasonable values and will protect the sample from indefinite waits.
api/Kotlin-Lab.api (4)
18329-18333: LGTM: Clean, coroutine-first API surface for KtorGitHubApiConstructor injects HttpClient and exposes suspend-style methods for org repos and contributors. This keeps transport concerns decoupled from API usage. No changes requested here.
18537-18549: LGTM: Repo model public API is minimal and serializableData class and serializer surface look correct after package refactor. No issues spotted.
18597-18609: LGTM: User model API is concise and serializableNo issues spotted in the new package surface.
18637-18666: All RequestData usages/imports updated to dev.shtanko.api.model — no stale references foundConfirmed via repo search: every import/occurrence references dev.shtanko.api.model.RequestData and the class is declared at src/main/kotlin/dev/shtanko/api/model/RequestData.kt.
Files touching RequestData (high level):
- src/main/kotlin/dev/shtanko/api/model/RequestData.kt (declaration)
- src/main/kotlin/dev/shtanko/api/tasks/BlockingRequest.kt
- src/main/kotlin/dev/shtanko/api/tasks/CallbackRequest.kt
- src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsBackground.kt
- src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsChannels.kt
- src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsConcurrent.kt
- src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsNotCancellable.kt
- src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsProgress.kt
- src/main/kotlin/dev/shtanko/api/tasks/LoadContributorsSuspend.kt
- src/main/kotlin/dev/shtanko/api/contributors/Logger.kt
- src/main/kotlin/dev/shtanko/api/contributors/Contributors.kt
- src/test/kotlin/dev/shtanko/api/ModelsTest.kt
- src/test/kotlin/dev/shtanko/api/contributors/TestData.kt
- api/Kotlin-Lab.api (API listing references)
No action required.
| public final class dev/shtanko/api/KtorInterceptorExampleKt { | ||
| public static final fun main ()V | ||
| public static synthetic fun main ([Ljava/lang/String;)V | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure example code never logs secrets (Authorization/API keys) and uses redacted logging
Examples are frequently copied verbatim. Please verify the example doesn’t print keys/tokens or raw Authorization headers and that any logging plugin in the example is configured to redact sensitive headers.
Run to double-check:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 -i 'println|print\(' src/main/kotlin/dev/shtanko/api/ktor src/main/kotlin/dev/shtanko/api | sed 's/^/HINT: /'
rg -n -C2 -i 'Authorization|X-Api-Key|Api-?Key' src/main/kotlin/dev/shtanko/api/ktor src/main/kotlin/dev/shtanko/api | sed 's/^/HINT: /'Length of output: 18416
Do not log secrets — redact Authorization/API keys and stop printing raw headers/bodies
Found plaintext logging in example interceptors that can leak Authorization/API keys and response bodies. Please fix before these examples are copied.
Files/locations to change (from verification run):
- src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt (lines ~27–31)
- Appends plugin.apiKey to requests and prints: println("🔑 Added API key header: ${plugin.headerName}")
- Action: remove/replace the println and never print the key or raw header value. If logging is necessary, log only that a key was added and that its value is redacted.
- src/main/kotlin/dev/shtanko/api/ktor/HttpLogInterceptor.kt (lines ~38–41, ~51–53)
- Prints request/response headers via context.headers.entries() and response.headers.entries().
- Action: do NOT log headers directly. Filter/map headers to redact sensitive names (HttpHeaders.Authorization, "X-API-Key", "Api-Key", etc.) before logging.
- src/main/kotlin/dev/shtanko/api/KtorInterceptorExample.kt (lines ~41–43, 61, 70–71, 75)
- Example installs ApiKeyInterceptor with apiKey = "your-api-key-here" and headerName = "Authorization" and prints response bodies/statuss (bodyAsText()).
- Action: remove full body prints or show only a truncated/sanitized summary; avoid printing or hardcoding real tokens in examples.
- src/main/kotlin/dev/shtanko/api/KtorGithubService.kt (line ~39)
- Sets header(HttpHeaders.Authorization, authToken) in defaultRequest — ensure any logging for this client redacts Authorization.
- src/main/kotlin/dev/shtanko/api/ktor/RetryInterceptor.kt (lines ~51, ~57)
- Prints retry attempts and exception messages. Prefer structured logging and avoid printing exception data that may include secrets.
Minimal recommended fixes:
- Replace printlns that expose headers/bodies with a helper that redacts sensitive headers, e.g. map header values to "" for Authorization/X-API-Key before logging.
- Remove printing of response.bodyAsText() in examples or log only a safe summary.
- Replace ad-hoc printlns with a logging API (or clearly-marked example helper) that documents which headers are redacted.
- Update example config to use an explicit empty/placeholder apiKey and callout that values must not be logged.
Please make these changes (or confirm redaction is implemented) and re-run the grep to ensure no other println/print calls expose sensitive headers or bodies.
🤖 Prompt for AI Agents
In api/Kotlin-Lab.api around lines 18340 to 18343, the example interceptors and
client code log sensitive data (API keys, Authorization headers, raw
request/response headers and bodies); update the listed Kotlin source files to
redact secrets: remove any printlns that print raw keys or bodies, replace them
with a logging helper that only logs that a header was added (e.g., "API key
header added") and never the key value, filter/map headers before logging to
replace values for sensitive names (HttpHeaders.Authorization, "X-API-Key",
"Api-Key", etc.) with "<REDACTED>", stop printing response.bodyAsText() (log
only a truncated/sanitized summary or status), avoid hardcoding example tokens
(use empty/placeholder constant and document they must not be logged), and
switch ad-hoc printlns to structured logging or a clearly-marked example helper;
apply these changes in src/main/kotlin/dev/shtanko/api/ktor/ApiKeyInterceptor.kt
(remove println of key), HttpLogInterceptor.kt (redact header values before
logging), KtorInterceptorExample.kt (remove body prints and hardcoded token),
KtorGithubService.kt (ensure Authorization is never logged), and
RetryInterceptor.kt (avoid printing full exception messages that may contain
secrets), then re-run the grep to confirm no other println/print exposes
secrets.
| public final class dev/shtanko/api/ktor/RequestResponseProcessorInterceptor { | ||
| public static final field Companion Ldev/shtanko/api/ktor/RequestResponseProcessorInterceptor$Companion; | ||
| public synthetic fun <init> (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/internal/DefaultConstructorMarker;)V | ||
| } | ||
|
|
||
| public final class dev/shtanko/api/ktor/RequestResponseProcessorInterceptor$Companion : io/ktor/client/plugins/HttpClientPlugin { | ||
| public fun getKey ()Lio/ktor/util/AttributeKey; | ||
| public fun install (Ldev/shtanko/api/ktor/RequestResponseProcessorInterceptor;Lio/ktor/client/HttpClient;)V | ||
| public synthetic fun install (Ljava/lang/Object;Lio/ktor/client/HttpClient;)V | ||
| public fun prepare (Lkotlin/jvm/functions/Function1;)Ldev/shtanko/api/ktor/RequestResponseProcessorInterceptor; | ||
| public synthetic fun prepare (Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; | ||
| } | ||
|
|
||
| public final class dev/shtanko/api/ktor/RequestResponseProcessorInterceptor$Config { | ||
| public fun <init> ()V | ||
| public final fun getRequestProcessor ()Lkotlin/jvm/functions/Function1; | ||
| public final fun getResponseProcessor ()Lkotlin/jvm/functions/Function2; | ||
| public final fun setRequestProcessor (Lkotlin/jvm/functions/Function1;)V | ||
| public final fun setResponseProcessor (Lkotlin/jvm/functions/Function2;)V | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make processors suspend and typed to Ktor primitives; guard against blocking and exceptions
Config uses plain Function1/Function2, which limits usage in coroutine pipelines. Prefer suspend function types and concrete Ktor types to enable async processing and proper context.
Patch src/main/kotlin/dev/shtanko/api/ktor/RequestResponseProcessorInterceptor.kt:
class RequestResponseProcessorInterceptor {
class Config {
- var requestProcessor: (Any) -> Unit = {}
- var responseProcessor: (Any, Any) -> Unit = { _, _ -> }
+ var requestProcessor: suspend (io.ktor.client.request.HttpRequestBuilder) -> Unit = {}
+ var responseProcessor: suspend (io.ktor.client.statement.HttpResponse, ByteArray?) -> Unit = { _, _ -> }
}
companion object : HttpClientPlugin<Config, RequestResponseProcessorInterceptor> {
override fun install(plugin: RequestResponseProcessorInterceptor, scope: HttpClient) {
- scope.sendPipeline.intercept(HttpSendPipeline.Monitoring) { plugin.requestProcessor(context); proceed() }
- scope.receivePipeline.intercept(HttpReceivePipeline.After) { plugin.responseProcessor(context.response, null); proceed() }
+ scope.sendPipeline.intercept(HttpSendPipeline.Monitoring) { plugin.requestProcessor(context); proceed() }
+ scope.receivePipeline.intercept(HttpReceivePipeline.After) { plugin.responseProcessor(context.response, null); proceed() }
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public final class dev/shtanko/api/ktor/RequestResponseProcessorInterceptor { | |
| public static final field Companion Ldev/shtanko/api/ktor/RequestResponseProcessorInterceptor$Companion; | |
| public synthetic fun <init> (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/internal/DefaultConstructorMarker;)V | |
| } | |
| public final class dev/shtanko/api/ktor/RequestResponseProcessorInterceptor$Companion : io/ktor/client/plugins/HttpClientPlugin { | |
| public fun getKey ()Lio/ktor/util/AttributeKey; | |
| public fun install (Ldev/shtanko/api/ktor/RequestResponseProcessorInterceptor;Lio/ktor/client/HttpClient;)V | |
| public synthetic fun install (Ljava/lang/Object;Lio/ktor/client/HttpClient;)V | |
| public fun prepare (Lkotlin/jvm/functions/Function1;)Ldev/shtanko/api/ktor/RequestResponseProcessorInterceptor; | |
| public synthetic fun prepare (Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; | |
| } | |
| public final class dev/shtanko/api/ktor/RequestResponseProcessorInterceptor$Config { | |
| public fun <init> ()V | |
| public final fun getRequestProcessor ()Lkotlin/jvm/functions/Function1; | |
| public final fun getResponseProcessor ()Lkotlin/jvm/functions/Function2; | |
| public final fun setRequestProcessor (Lkotlin/jvm/functions/Function1;)V | |
| public final fun setResponseProcessor (Lkotlin/jvm/functions/Function2;)V | |
| } | |
| // src/main/kotlin/dev/shtanko/api/ktor/RequestResponseProcessorInterceptor.kt | |
| package dev.shtanko.api.ktor | |
| import io.ktor.client.HttpClient | |
| import io.ktor.client.plugins.HttpClientPlugin | |
| import io.ktor.client.request.HttpRequestBuilder | |
| import io.ktor.client.request.HttpSendPipeline | |
| import io.ktor.client.statement.HttpReceivePipeline | |
| import io.ktor.client.statement.HttpResponse | |
| import io.ktor.util.AttributeKey | |
| class RequestResponseProcessorInterceptor private constructor( | |
| private val requestProcessor: suspend (HttpRequestBuilder) -> Unit, | |
| private val responseProcessor: suspend (HttpResponse, ByteArray?) -> Unit | |
| ) { | |
| class Config { | |
| var requestProcessor: suspend (io.ktor.client.request.HttpRequestBuilder) -> Unit = {} | |
| var responseProcessor: suspend (io.ktor.client.statement.HttpResponse, ByteArray?) -> Unit = { _, _ -> } | |
| } | |
| companion object : HttpClientPlugin<Config, RequestResponseProcessorInterceptor> { | |
| override val key: AttributeKey<RequestResponseProcessorInterceptor> = | |
| AttributeKey("RequestResponseProcessorInterceptor") | |
| override fun prepare(block: Config.() -> Unit): RequestResponseProcessorInterceptor { | |
| val config = Config().apply(block) | |
| return RequestResponseProcessorInterceptor(config.requestProcessor, config.responseProcessor) | |
| } | |
| override fun install(plugin: RequestResponseProcessorInterceptor, scope: HttpClient) { | |
| scope.sendPipeline.intercept(HttpSendPipeline.Monitoring) { | |
| // subject here is the HttpRequestBuilder | |
| plugin.requestProcessor(context) | |
| proceed() | |
| } | |
| scope.receivePipeline.intercept(HttpReceivePipeline.After) { | |
| // context.response is the HttpResponse | |
| plugin.responseProcessor(context.response, null) | |
| proceed() | |
| } | |
| } | |
| } | |
| } |
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores
Tests