KTOR-822 Add full implementation of the HttpBin sample#332
KTOR-822 Add full implementation of the HttpBin sample#332
Conversation
WalkthroughRefactors the httpbin sample into modular Ktor route modules, replaces Gson with kotlinx.serialization, adds Brotli/native resolution and OpenAPI/Swagger support, implements custom Basic/Digest auth providers, and introduces extensive new tests while removing the previous monolithic application and many static assets. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/BasicAuth.kt`:
- Around line 136-158: The basicAuthenticationCredentials function ignores the
charset parameter and always decodes bytes as UTF-8; update it to use the
provided charset (or ISO-8859-1 when charset == null for legacy RFC 7617
behavior) when converting decoded Base64 bytes to String, and remove the
redundant "0 + bytes.size" usage—i.e., determine val effectiveCharset = charset
?: Charsets.ISO_8859_1 and create the userPass string from the bytes using that
charset before finding the colon and returning UserPasswordCredential in
basicAuthenticationCredentials (keep the existing parseAuthorizationHeader /
HttpAuthHeader.Single checks and error handling).
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/DigestAuth.kt`:
- Around line 239-250: The toDigestCredential function should stop using the
non-null asserted (!!) parameter() calls and instead validate required fields
(realm, username, uri, nonce, response) and return null if any are missing;
change its signature to return DigestCredential? and construct a
DigestCredential only when all required parameters are present, passing optional
parameters as before. Update call sites that invoke toDigestCredential (notably
onAuthenticate and the other usage around the existing toDigestCredential call)
to handle a nullable return by treating null as missing/invalid credentials and
proceeding accordingly. Ensure no other code assumes a non-null result from
toDigestCredential and add null checks or early returns where needed.
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Dynamic.kt`:
- Around line 331-347: The /stream/{n} handler currently lets "number" grow
unbounded (val number = ...coerceAtLeast(0)), which can lead to DoS; update the
parsing logic in the get("/stream/{n}") route to enforce a maximum (e.g., a
STREAM_MAX constant set to 100 * 1024) and apply .coerceIn(0, STREAM_MAX) or
.coerceAtMost(STREAM_MAX) when computing "number" before building responses;
keep using the same HttpbinResponse.Builder and call.respondTextWriter loop but
iterate only up to the capped value and consider making STREAM_MAX configurable
if needed.
- Around line 240-271: The /range/{numbytes} route builds an unbounded string
from the requested number and lacks the same 102400 cap as other endpoints;
update the validation for the variable number (obtained from
call.parameters["numbytes"]) to enforce an upper bound (e.g., MAX_BYTES = 100 *
1024) — either by using coerceIn(1, MAX_BYTES) or by returning BadRequest when
number is <= 0 or > MAX_BYTES — and update the error message text to correctly
reference the enforced upper bound; adjust any uses of number (body construction
and ETag) accordingly in the route("/range/{numbytes}") handler.
- Around line 153-155: timeMillis() currently returns Float causing precision
loss; change it to return a higher-precision type (preferably Double or Long)
and update any callers accordingly: modify timeMillis() to compute milliseconds
as either System.nanoTime() / 1_000_000.0 and return Double, or
System.nanoTime() / 1_000_000L and return Long, then update drip (and its
parameter type if it accepts a time supplier) to accept the same type (() ->
Double or () -> Long) and adjust the subtraction/interval arithmetic in drip to
use Double/Long arithmetic; ensure all usages of timeMillis() and drip use the
new signature and types.
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/images.kt`:
- Around line 18-33: The Accept header parsing in the block that builds
clientAccepts (from call.request.accept()) should strip any media type
parameters (e.g., ;q=0.9) before matching against serverAccepts so quality
params don't prevent matches; change the clientAccepts mapping to split each
entry on ';' and take the media type token (trimmed, lowercased) or use Ktor's
Accept header utilities, then compute accepts =
clientAccepts.intersect(serverAccepts) as before and return the same
ImageErrorResponse when empty.
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Responses.kt`:
- Around line 245-248: The Kotlin code uses guarded when branches (the
expression starting with "when (val port = request.local.serverPort)" that
contains branches like "80 if request.local.scheme == \"http\" ->") which
require the -Xwhen-guards compiler flag; to fix, add the flag to the Kotlin
compiler options in the module build configuration (httpbin/build.gradle.kts) by
adding "-Xwhen-guards" to kotlin.compilerOptions.freeCompilerArgs so the guarded
when syntax compiles.
In `@httpbin/src/test/kotlin/io/ktor/samples/httpbin/DynamicDataTest.kt`:
- Around line 462-476: The test streamBytesChuckSize calls /stream-bytes with
the unsupported query param name chunk-size (hyphen) so the server ignores it;
update the request in the streamBytesChuckSize test (the
client.get("/stream-bytes/…") call) to use chunk_size=333 (underscore) to match
the endpoint parsing in Dynamic.kt and ensure the 333-byte chunk-size is
actually exercised.
- Around line 554-571: The call to the non-existent runTestApplication causes a
compile error; replace it with the Ktor testing helper testApplication and keep
the virtual-time setup: inside runTest(disp) call testApplication { engine {
dispatcher = disp } and then invoke the existing block(scheduler) within that
lambda. Update references to runTestApplication -> testApplication and ensure
you still pass the dispatcher into engine and call block(scheduler) from the
ApplicationTestBuilder context.
In `@httpbin/src/test/kotlin/io/ktor/samples/httpbin/ResponseFormatTest.kt`:
- Around line 191-202: The isValidUtf8 function catches CharacterCodingException
but the class isn't imported; add the missing import for
java.nio.charset.CharacterCodingException to the file so the catch clause
compiles and the isValidUtf8(ByteArray) function resolves correctly.
🟡 Minor comments (19)
httpbin/src/test/kotlin/io/ktor/samples/httpbin/UnsafeMethodsBodyTest.kt-388-395 (1)
388-395:⚠️ Potential issue | 🟡 MinorTypo:
"tes3"→"test3".Line 390 has
"tes3"which looks like a typo for"test3", given the pattern of"test2"and"test1"in the adjacent entries. While it doesn't affect test correctness (only key ordering is asserted), it's a distracting inconsistency.🧹 Fix typo
- append("p3", "tes3", Headers.build { + append("p3", "test3", Headers.build {httpbin/src/test/kotlin/io/ktor/samples/httpbin/UnsafeMethodsBodyTest.kt-251-255 (1)
251-255:⚠️ Potential issue | 🟡 MinorTrailing whitespace in the JSON body string literal.
Line 253 has a trailing space after the closing
}. AftertrimIndent(), the request body will be{"name": {}, ...null}(note the trailing space). While most JSON parsers tolerate this, it appears unintentional.🧹 Remove trailing space
val body = client.post("/post") { setBody(""" - {"name": {}, "values": [1, 2, 3], "flag": true, "nothing": null} + {"name": {}, "values": [1, 2, 3], "flag": true, "nothing": null} """.trimIndent()) }.body<JsonObject>()httpbin/src/main/kotlin/io/ktor/samples/httpbin/images.kt-52-52 (1)
52-52:⚠️ Potential issue | 🟡 MinorTypo: "suggest" → "suggested".
"Returns a simple image of the type suggest by the Accept header."should read"…suggested by…".httpbin/src/test/kotlin/io/ktor/samples/httpbin/StatusCodesTest.kt-19-19 (1)
19-19:⚠️ Potential issue | 🟡 MinorStray assertion: HEAD /get is unrelated to status codes.
This line tests
AutoHeadResponsebehavior, not the/status/{codes}endpoint. Consider moving it to a more appropriate test class (e.g., a methods or request-inspection test).httpbin/settings.gradle.kts-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorDocument the fragility of using Gradle's internal
DefaultTargetMachineFactoryAPI.
org.gradle.nativeplatform.internal.DefaultTargetMachineFactoryis not part of Gradle's public API and may break on Gradle upgrades. There is no stable public alternative for detecting the host OS/architecture at settings-evaluation time—the publicTargetMachineFactoryAPI is designed for target machine construction, not host detection. Add a comment explaining this dependency on the internal API so future maintainers are aware of the fragility and can monitor for breakage across Gradle versions.httpbin/src/main/kotlin/io/ktor/samples/httpbin/Cookies.kt-50-60 (1)
50-60:⚠️ Potential issue | 🟡 MinorOpenAPI response status mismatch: endpoint returns 302 but describes 200.
The
/cookies/deleteendpoint responds withHttpStatusCode.Found(302), but thedescribeblock documentsHttpStatusCode.OK(200) with aCookiesResponseschema. The same issue applies to/cookies/set(line 95) and/cookies/set/{name}/{value}(line 121). Consumers of the generated OpenAPI spec will not see the redirect behavior.Consider documenting the actual 302 response (with a
Locationheader) and optionally a 200 for the final redirect target.httpbin/src/main/kotlin/io/ktor/samples/httpbin/Anything.kt-1-41 (1)
1-41:⚠️ Potential issue | 🟡 MinorMissing trailing newline.
detekt flags this file for not ending with a newline character. Add a newline at the end of the file.
httpbin/src/main/kotlin/io/ktor/samples/httpbin/RequestInspection.kt-43-43 (1)
43-43:⚠️ Potential issue | 🟡 MinorTypo: "requests's" → "request's".
✏️ Fix
- summary = "Return the incoming requests's User-Agent header." + summary = "Return the incoming request's User-Agent header."httpbin/src/main/kotlin/io/ktor/samples/httpbin/ResponseFormats.kt-65-74 (1)
65-74:⚠️ Potential issue | 🟡 MinorCopy-paste bug: OpenAPI summary for
/deflatesays "Brotli-encoded".Line 67 reads
"Returns Brotli-encoded data."but this is the/deflateendpoint. The description on line 70 is correct ("Deflate-encoded data."), but the summary was not updated.🐛 Fix the summary text
- summary = "Returns Brotli-encoded data." + summary = "Returns Deflate-encoded data."httpbin/src/test/kotlin/io/ktor/samples/httpbin/ResponseFormatTest.kt-42-42 (1)
42-42:⚠️ Potential issue | 🟡 Minor
assertEqualsarguments are swapped.
kotlin.test.assertEqualshas the signatureassertEquals(expected, actual). Here, the actual result is passed as the first argument and the expected constant as the second.🐛 Fix argument order
- assertEquals(directDecompress.resultStatus, DecoderJNI.Status.DONE) + assertEquals(DecoderJNI.Status.DONE, directDecompress.resultStatus)httpbin/src/main/kotlin/io/ktor/samples/httpbin/Auth.kt-127-127 (1)
127-127:⚠️ Potential issue | 🟡 MinorMissing trailing newline.
Detekt reports the file does not end with a newline.
httpbin/src/test/kotlin/io/ktor/samples/httpbin/DynamicDataTest.kt-462-467 (1)
462-467:⚠️ Potential issue | 🟡 MinorTypo in test name:
streamBytesChuckSize→streamBytesChunkSize.Proposed fix
- fun streamBytesChuckSize() = testApplication { + fun streamBytesChunkSize() = testApplication {httpbin/src/main/kotlin/io/ktor/samples/httpbin/ResponseInspection.kt-186-189 (1)
186-189:⚠️ Potential issue | 🟡 MinorFragile Content-Length self-reference calculation.
The formula
bytes.size + (bytes.size.toString().length - 1)assumes the digit count won't shift when adjusting. Ifbytes.sizeis at a boundary (e.g., 99), the computed length would be 100, but replacing"0"with"100"in the JSON adds 2 more characters (not 1), producing a 101-byte body with aContent-Length: 100header — an off-by-one.A safer approach is to iterate until the value stabilizes:
Proposed fix
- headers["Content-Length"] = listOf("0") - val bytes = prettyJson.encodeToString(SmartValueMapSerializer, headers).toByteArray() - headers["Content-Length"] = listOf((bytes.size + (bytes.size.toString().length - 1)).toString()) + var contentLength = "0" + repeat(3) { + headers["Content-Length"] = listOf(contentLength) + val size = prettyJson.encodeToString(SmartValueMapSerializer, headers).toByteArray().size + contentLength = size.toString() + } + headers["Content-Length"] = listOf(contentLength)httpbin/src/main/kotlin/io/ktor/samples/httpbin/ResponseInspection.kt-155-199 (1)
155-199:⚠️ Potential issue | 🟡 MinorDead code: the HTTP method check at Line 159 is unreachable.
The
for (method in listOf(HttpMethod.Get, HttpMethod.Post))loop withmethod(method)already constrains the handler to GET and POST. The condition on Line 159 will never be true.Proposed fix — remove the dead check
method(method) { handle { - if (call.request.httpMethod != HttpMethod.Get && call.request.httpMethod != HttpMethod.Post) { - call.respond(HttpStatusCode.NotFound) - return@handle - } - val headers = mutableMapOf<String, List<String>>()httpbin/src/main/kotlin/io/ktor/samples/httpbin/Server.kt-127-149 (1)
127-149:⚠️ Potential issue | 🟡 MinorUnvalidated
algorithmparameter can throwNoSuchAlgorithmException.
MessageDigest.getInstance(algorithm)(Line 138) will throw at runtime if a client supplies an unsupported algorithm in the path (e.g.,/digest-auth/user/pass/BOGUS), resulting in an unhandled 500 error. Consider validating against a whitelist of supported algorithms (e.g.,MD5,SHA-256,SHA-512).Proposed fix
+ val SUPPORTED_ALGORITHMS = setOf("MD5", "SHA-256", "SHA-512") + digestProvider { _, realm -> val user = parameters["user"] ?: return@digestProvider null val password = parameters["password"] ?: return@digestProvider null val algorithm = parameters["algorithm"] ?: "MD5" + if (algorithm !in SUPPORTED_ALGORITHMS) return@digestProvider null + MessageDigest.getInstance(algorithm).digest( "$user:$realm:$password".toByteArray() ) }httpbin/src/test/kotlin/io/ktor/samples/httpbin/ResponseInspectionTest.kt-228-229 (1)
228-229:⚠️ Potential issue | 🟡 MinorPotential NPE on
response.contentType().toString().Line 229 calls
response.contentType()without null-safety, but Line 225 usesresponse.contentType()?.withoutParameters()— acknowledging it could be null. If the response somehow lacks a Content-Type, Line 229 would throw. Consider using?.toString()or a non-null assertion with a message.Proposed fix
- assertEquals(response.contentType().toString(), body["Content-Type"].asString) + assertEquals(response.contentType()?.toString(), body["Content-Type"].asString)Similarly on Line 254.
httpbin/src/main/kotlin/io/ktor/samples/httpbin/ResponseInspection.kt-25-25 (1)
25-25:⚠️ Potential issue | 🟡 Minor
headerRegexdoes not conform to RFC 7230 HTTP token specification.The regex
[A-Za-z0-9-_]+only permits alphanumerics, hyphens, and underscores. However, RFC 7230 §3.2.6 definestcharto also include:!,#,$,%,&,',*,+,.,^,`,|, and~. This regex will reject valid HTTP header names. If the restricted character set is intentional for this sample, add a comment explaining the rationale.httpbin/src/main/kotlin/io/ktor/samples/httpbin/DigestAuth.kt-296-308 (1)
296-308:⚠️ Potential issue | 🟡 MinorNullable
nonceCountandcnonceinterpolated as literal"null"in hash.When
qopis non-null,nonceCountandcnonce(bothString?) are included in thejoinToString(":")list. If they arenull, Kotlin'stoString()produces the literal string"null", which will silently produce an incorrect digest rather than failing explicitly. Per RFC 2617, ifqopis present,ncandcnoncemust be present — but a malformed request could trigger this.httpbin/src/main/kotlin/io/ktor/samples/httpbin/DigestAuth.kt-108-114 (1)
108-114:⚠️ Potential issue | 🟡 MinorDefault
digestProvidercomputes hash without the password component.The default lambda computes
H(username:realm)instead of the standardH(username:realm:password). WhileServer.ktoverrides this with the correct formula, anyone relying on the default will silently get broken authentication.
🧹 Nitpick comments (30)
httpbin/src/test/kotlin/io/ktor/samples/httpbin/UnsafeMethodsBodyTest.kt (3)
22-46: Consider extracting common test setup to reduce boilerplate.Every test method repeats the identical
testApplication { application { module() } }block and client creation with GsonContentNegotiation. This is ~8 lines duplicated 16 times. Extracting a helper (e.g., a private function or a JUnit@BeforeTestsetup) would make tests more concise and easier to maintain.♻️ Example helper extraction
private fun testPost( path: String = "/post", block: io.ktor.client.request.HttpRequestBuilder.() -> Unit = {}, assertions: suspend (JsonObject) -> Unit ) = testApplication { application { module() } val client = createClient { install(ContentNegotiation) { gson() } } val body = client.post(path, block).body<JsonObject>() assertions(body) }Then each test becomes:
`@Test` fun noBody() = testPost { body -> assertTrue(body.has("data")) assertEquals("", body.get("data").asString) // ... }
224-239: Test sends"123"without an explicit content type — verify server behavior.
setBody("123")without acontentType(...)call will default totext/plain. The test asserts that the server parses this as JSON (body.get("json").asJsonPrimitive.asInt == 123). This works if the server-side code attempts JSON parsing of the raw body regardless of Content-Type, but the intent is subtle and undocumented. A brief comment clarifying this is intentional (testing that the server auto-detects JSON in plain text bodies) would improve readability.
48-70: Key ordering assertion is brittle.This test asserts the exact iteration order of JSON keys in the response. If the server-side response model ever adds, removes, or renames a field, this test will fail with a confusing
NoSuchElementExceptionfrom the iterator rather than a clear assertion message. Consider asserting against a list for a better failure message:♻️ Suggested improvement
- val keys = body.asJsonObject.keySet().iterator() - - assertEquals("args", keys.next()) - assertEquals("data", keys.next()) - assertEquals("files", keys.next()) - assertEquals("form", keys.next()) - assertEquals("headers", keys.next()) - assertEquals("json", keys.next()) - assertEquals("origin", keys.next()) - assertEquals("url", keys.next()) + assertEquals( + listOf("args", "data", "files", "form", "headers", "json", "origin", "url"), + body.asJsonObject.keySet().toList() + )httpbin/src/test/kotlin/io/ktor/samples/httpbin/RedirectsTest.kt (1)
13-237: Consider extracting the repeated test setup into a helper.Every test method repeats the same
testApplication { application { module() }; createClient { followRedirects = false } }boilerplate. For a sample project this is acceptable for readability, but a small helper would cut ~100 lines of repetition and make it easier to add new cases.♻️ Example helper extraction
private fun redirectTest(block: suspend ApplicationTestBuilder.(HttpClient) -> Unit) = testApplication { application { module() } val client = createClient { followRedirects = false } block(client) }Then each test becomes:
`@Test` fun absolute() = redirectTest { client -> val response = client.get("/absolute-redirect/0") assertEquals(HttpStatusCode.BadRequest, response.status) }httpbin/src/main/kotlin/io/ktor/samples/httpbin/Statuses.kt (1)
34-34: Clarify the random selection after validation.After the validation guard,
statusCodescannot containnulls, so thefilterNotNull()call is only needed for the type-cast. Consider making the intent clearer:♻️ Suggested improvement
- val code = statusCodes.filterNotNull()[random.nextInt(statusCodes.size)] + val validCodes = statusCodes.filterNotNull() + val code = validCodes[random.nextInt(validCodes.size)]This avoids the subtle reliance on both lists having the same size and makes the intent explicit.
httpbin/src/main/kotlin/io/ktor/samples/httpbin/Redirects.kt (2)
55-62: Inconsistent query parameter access.Line 55 uses
call.queryParameters["url"]while Line 62 usescall.request.queryParameters["status_code"]. Both resolve identically in Ktor, but mixing the two forms in the same handler is needlessly inconsistent.♻️ Align to the shorter form
- var statusCode = call.request.queryParameters["status_code"]?.toIntOrNull() ?: 302 + var statusCode = call.queryParameters["status_code"]?.toIntOrNull() ?: 302
28-29:requirewill crash the server on empty connectors.
require(connectors.isNotEmpty())throwsIllegalArgumentException, which will surface as an unhandled 500 to the client. For a sample app this is acceptable, but responding with a meaningful error would be more robust.httpbin/src/test/kotlin/io/ktor/samples/httpbin/StatusCodesTest.kt (1)
33-111: Significant test duplication across HTTP methods.The
post(),put(),patch(), anddelete()tests are near-identical copies ofget()(minus the HEAD assertion). This could be parameterized to reduce maintenance burden.♻️ Example: parameterized approach
private fun statusTest( method: suspend HttpClient.(String) -> HttpResponse ) = testApplication { application { module() } assertEquals(HttpStatusCode.OK, client.method("/status/200").status) assertEquals(HttpStatusCode.NotFound, client.method("/status/404").status) assertEquals(HttpStatusCode.InternalServerError, client.method("/status/500").status) val noRedirectClient = createClient { followRedirects = false } for (code in listOf("301", "302", "303", "307")) { assertEquals("/redirect/1", noRedirectClient.method("/status/$code").headers["Location"]) } } `@Test` fun get() = statusTest { get(it) } `@Test` fun post() = statusTest { post(it) } // ...httpbin/src/main/kotlin/io/ktor/samples/httpbin/Responses.kt (1)
165-165:limitedParallelism(64)creates a new dispatcher view perBuilderinstance.Each
Builderinstantiation allocates a new limited-parallelism wrapper aroundDispatchers.IO. Move this to a companion object (or top-levelval) so a single view is shared:♻️ Suggested fix
class Builder { + companion object { + private val ioDispatcher = Dispatchers.IO.limitedParallelism(64) + } + // ... - private val ioDispatcher = Dispatchers.IO.limitedParallelism(64)httpbin/src/main/kotlin/io/ktor/samples/httpbin/OpenAPI.kt (1)
79-118:partialSchemaandschemaWithExamplesshare near-identical iteration logic.The only difference is the
p in propNamesfilter. Consider havingschemaWithExamplesdelegate topartialSchemato avoid the duplication:♻️ Optional consolidation
inline fun <reified T : Any> Response.Builder.schemaWithExamples(title: String): JsonSchema { - val fullSchema = jsonSchema<T>() - - val props = mutableMapOf<String, ReferenceOr<JsonSchema>>() - for ((p, v) in fullSchema.properties ?: emptyMap()) { - if (ALL_EXAMPLES.containsKey(p)) { - props[p] = v.mapValue { it.copy(example = ALL_EXAMPLES[p]) } - } else { - props[p] = v - } - } - - return fullSchema.copy( - title = title, - properties = props, - ) + val fullSchema = jsonSchema<T>() + val allPropNames = (fullSchema.properties ?: emptyMap()).keys.toTypedArray() + return partialSchema<T>(title, *allPropNames) }httpbin/src/main/kotlin/io/ktor/samples/httpbin/Cookies.kt (1)
1-127: Missing trailing newline and function length.detekt flags two issues: the file doesn't end with a newline, and
cookies()exceeds the 60-line maximum (98 lines). The newline is a quick fix. For the length, consider splitting into helper functions (e.g.,cookiesGet,cookiesDelete,cookiesSet) if you want to satisfy the rule, though it's understandable for a sample project.httpbin/src/test/kotlin/io/ktor/samples/httpbin/AnythingTest.kt (1)
15-15: Inconsistent@Testimport:org.junit.Testvskotlin.test.Test.This file imports
org.junit.Testwhile the other test files in this PR (e.g.,OriginTest.kt,MethodsTest.kt,RequestedURLTest.kt,HeadersTest.kt) usekotlin.test.Test. Consider switching tokotlin.test.Testfor consistency.Proposed fix
-import org.junit.Test +import kotlin.test.Testhttpbin/src/main/kotlin/io/ktor/samples/httpbin/ResponseFormats.kt (2)
191-200: Grammar nit: "An JSON" → "A JSON".Line 196:
"An JSON document."should be"A JSON document."since "JSON" starts with a consonant sound.✏️ Fix
- description = "An JSON document." + description = "A JSON document."
26-49:Brotli4jLoader.ensureAvailability()is called on every request.This call verifies native library availability and should ideally be done once at startup rather than on each
/brotlirequest. In the context of a sample app this is unlikely to be a real problem, but it's worth noting.httpbin/src/main/resources/utf8.html (1)
4-4: Links downgraded from HTTPS to HTTP.The href and displayed URLs were changed from
https://tohttp://. Prefer HTTPS where the target supports it to avoid mixed-content warnings and to preserve link integrity for users who click through.httpbin/src/main/kotlin/io/ktor/samples/httpbin/Dynamic.kt (1)
64-69: Silent empty response on invalid/bytes/{numbytes}input.When
nis null (non-integer) or negative,return@getproduces an empty 200 OK response with no body, which is confusing for the caller. Consider responding with a 400 status like the seed validation does on line 78.♻️ Return an explicit error
val n = call.parameters["numbytes"]?.toIntOrNull() if (n == null || n < 0) { - return@get + call.respondText("Invalid number of bytes.", status = HttpStatusCode.BadRequest) + return@get }httpbin/src/test/kotlin/io/ktor/samples/httpbin/ArgsTest.kt (1)
14-97: Consider extracting the repeated client setup.Every test method creates an identical
createClient { install(ContentNegotiation) { gson() } }block. This is repeated across this file and other test files (e.g.,RequestInspectionTest.kt). A shared helper or a base class could reduce boilerplate.httpbin/src/test/kotlin/io/ktor/samples/httpbin/RequestInspectionTest.kt (1)
116-116: Nit: missing space before=.- fun userAgent()= testApplication { + fun userAgent() = testApplication {httpbin/src/test/kotlin/io/ktor/samples/httpbin/CookiesTest.kt (1)
21-247: Consider extracting the repeatedcreateClient { install(ContentNegotiation) { gson() } }pattern.This exact client setup is duplicated in 5 out of 12 tests in this file (and across many other test files). A shared helper or a class-level property could reduce boilerplate, e.g.:
private fun ApplicationTestBuilder.gsonClient() = createClient { install(ContentNegotiation) { gson() } }httpbin/src/main/kotlin/io/ktor/samples/httpbin/ResponseInspection.kt (1)
46-46: Simplify timestamp creation.
GMTDate(Date.from(Instant.now()).time)does a redundantInstant → Date → Longround-trip. Ktor'sGMTDate()(no-arg) already returns the current time.Proposed fix
- val gmt = GMTDate(Date.from(Instant.now()).time) + val gmt = GMTDate()Also applies to: 91-91
httpbin/src/main/kotlin/io/ktor/samples/httpbin/Auth.kt (1)
14-127: Functionauth()exceeds detekt's max length (105 vs 60 lines).Each
authenticateblock follows the same pattern. Consider extracting per-scheme helpers (e.g.,basicAuth(),bearerAuth(),digestAuth()) to reduce length and improve navigability.httpbin/src/main/kotlin/io/ktor/samples/httpbin/Server.kt (1)
43-46:anyHost()withallowCredentials = trueis a permissive CORS configuration.This is fine for a sample/demo application, but worth a brief inline comment noting this should not be used in production. Browsers will reflect any origin with credentials allowed.
httpbin/src/test/kotlin/io/ktor/samples/httpbin/AuthTest.kt (2)
3-3: Server uses kotlinx.serialization JSON, but tests deserialize with Gson.The server module installs
ContentNegotiation { json(prettyJson) }(kotlinx.serialization) while the test client uses Gson for deserialization. This works because both produce/consume standard JSON, but it means the test doesn't actually validate the serialization library the server uses. If response shapes ever diverge due to serialization-library-specific behavior (e.g., nulls handling, number types), Gson on the client side could mask it.Not a blocker for a sample project, but worth noting.
25-36: Consider extracting the repeated test client setup.Every test repeats the same
createClient { install(ContentNegotiation) { gson() } }block. Extracting a helper (or using a setup function) would reduce boilerplate and make it easier to switch the deserialization library in the future.♻️ Example helper
private fun ApplicationTestBuilder.jsonClient(block: HttpClientConfig<*>.() -> Unit = {}) = createClient { install(ContentNegotiation) { gson() } block() }Then tests become:
val client = jsonClient() // or with extra config: val client = jsonClient { install(Auth) { digest { ... } } }Also applies to: 39-54, 57-75, 78-96, 99-111, 114-128, 131-149, 152-175, 178-202, 205-226, 229-254, 257-268, 271-289
httpbin/src/main/kotlin/io/ktor/samples/httpbin/BasicAuth.kt (3)
143-148: CatchingThrowableis overly broad.Base64 decoding failures typically throw
IllegalArgumentException. CatchingThrowablecould swallow critical errors likeOutOfMemoryErrororThreadDeath.♻️ Narrow the catch clause
val userPass = try { val bytes = authHeader.blob.decodeBase64Bytes() - bytes.decodeToString(0, 0 + bytes.size) - } catch (e: Throwable) { + bytes.toString(charset ?: Charsets.ISO_8859_1) + } catch (e: Exception) { return null }
160-160: Useprivate const valfor the challenge key.Since
basicAuthenticationChallengeKeyis aStringliteral assigned toAny, it can be simplified to aconst valwith an explicitStringtype.♻️ Proposed fix
-private val basicAuthenticationChallengeKey: Any = "BasicAuth" +private const val basicAuthenticationChallengeKey: String = "BasicAuth"
4-4: Remove unused/conflicting imports.
- Line 4:
io.ktor.http.auth.HttpAuthHeader.Companionis redundant—the wildcard import on line 3 already imports all exports fromio.ktor.http.auth.- Line 14:
io.ktor.server.auth.basicis shadowed by the locally definedbasicextension function (line 123) and is never used.♻️ Remove unused imports
-import io.ktor.http.auth.HttpAuthHeader.Companion import io.ktor.server.application.* import io.ktor.server.auth.Authentication import io.ktor.server.auth.AuthenticationConfig import io.ktor.server.auth.AuthenticationContext import io.ktor.server.auth.AuthenticationFailedCause import io.ktor.server.auth.AuthenticationFunction import io.ktor.server.auth.AuthenticationProvider import io.ktor.server.auth.UnauthorizedResponse import io.ktor.server.auth.UserPasswordCredential -import io.ktor.server.auth.basic import io.ktor.server.auth.parseAuthorizationHeaderhttpbin/src/main/kotlin/io/ktor/samples/httpbin/DigestAuth.kt (3)
130-132: Commented-out code and missing visibility modifier.Line 130 contains commented-out dead code. Line 132
getAlgorithmlacks an explicit visibility modifier, defaulting topublic, while other properties in the same class use explicitpublicannotations. Consider removing the dead code and addingpublic(orinternalif it shouldn't be part of the public API).♻️ Proposed fix
-// public var algorithmName: String = "MD5" - - var getAlgorithm: ApplicationCall.() -> String = { "MD5" } + public var getAlgorithm: ApplicationCall.() -> String = { "MD5" }
14-14: Unused import of built-indigest.
io.ktor.server.auth.digestis the built-in digest extension that is shadowed by the localdigestextension on Line 178. This import is unused and confusing.♻️ Remove unused import
-import io.ktor.server.auth.digest
232-232: Useconst valfor the challenge key.Same as in
BasicAuth.kt— this is a compile-time constantString.♻️ Proposed fix
-private val digestAuthenticationChallengeKey: Any = "DigestAuth" +private const val digestAuthenticationChallengeKey: String = "DigestAuth"
| public fun ApplicationRequest.basicAuthenticationCredentials(charset: Charset? = null): UserPasswordCredential? { | ||
| when (val authHeader = parseAuthorizationHeader()) { | ||
| is HttpAuthHeader.Single -> { | ||
| // Verify the auth scheme is HTTP Basic. According to RFC 2617, the authorization scheme should not be | ||
| // case-sensitive; thus BASIC, or Basic, or basic are all valid. | ||
| if (!authHeader.authScheme.equals("Basic", ignoreCase = true)) return null | ||
|
|
||
| val userPass = try { | ||
| val bytes = authHeader.blob.decodeBase64Bytes() | ||
| bytes.decodeToString(0, 0 + bytes.size) | ||
| } catch (e: Throwable) { | ||
| return null | ||
| } | ||
|
|
||
| val colonIndex = userPass.indexOf(':') | ||
|
|
||
| if (colonIndex == -1) return null | ||
|
|
||
| return UserPasswordCredential(userPass.substring(0, colonIndex), userPass.substring(colonIndex + 1)) | ||
| } | ||
| else -> return null | ||
| } | ||
| } |
There was a problem hiding this comment.
charset parameter is accepted but never used in credential decoding.
The charset parameter controls whether RFC 7617 UTF-8 mode or legacy ISO-8859-1 mode is used for decoding the Base64 payload. However, Line 145 always uses decodeToString() which defaults to UTF-8, ignoring the charset argument. When charset is null (legacy mode, as configured in Server.kt with charset = null), the decoded bytes should be interpreted as ISO-8859-1.
Also, 0 + bytes.size on Line 145 is redundant.
🐛 Proposed fix
val userPass = try {
val bytes = authHeader.blob.decodeBase64Bytes()
- bytes.decodeToString(0, 0 + bytes.size)
+ bytes.toString(charset ?: Charsets.ISO_8859_1)
} catch (e: Throwable) {
return null
}📝 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 fun ApplicationRequest.basicAuthenticationCredentials(charset: Charset? = null): UserPasswordCredential? { | |
| when (val authHeader = parseAuthorizationHeader()) { | |
| is HttpAuthHeader.Single -> { | |
| // Verify the auth scheme is HTTP Basic. According to RFC 2617, the authorization scheme should not be | |
| // case-sensitive; thus BASIC, or Basic, or basic are all valid. | |
| if (!authHeader.authScheme.equals("Basic", ignoreCase = true)) return null | |
| val userPass = try { | |
| val bytes = authHeader.blob.decodeBase64Bytes() | |
| bytes.decodeToString(0, 0 + bytes.size) | |
| } catch (e: Throwable) { | |
| return null | |
| } | |
| val colonIndex = userPass.indexOf(':') | |
| if (colonIndex == -1) return null | |
| return UserPasswordCredential(userPass.substring(0, colonIndex), userPass.substring(colonIndex + 1)) | |
| } | |
| else -> return null | |
| } | |
| } | |
| public fun ApplicationRequest.basicAuthenticationCredentials(charset: Charset? = null): UserPasswordCredential? { | |
| when (val authHeader = parseAuthorizationHeader()) { | |
| is HttpAuthHeader.Single -> { | |
| // Verify the auth scheme is HTTP Basic. According to RFC 2617, the authorization scheme should not be | |
| // case-sensitive; thus BASIC, or Basic, or basic are all valid. | |
| if (!authHeader.authScheme.equals("Basic", ignoreCase = true)) return null | |
| val userPass = try { | |
| val bytes = authHeader.blob.decodeBase64Bytes() | |
| bytes.decodeToString(charset ?: Charsets.ISO_8859_1) | |
| } catch (e: Throwable) { | |
| return null | |
| } | |
| val colonIndex = userPass.indexOf(':') | |
| if (colonIndex == -1) return null | |
| return UserPasswordCredential(userPass.substring(0, colonIndex), userPass.substring(colonIndex + 1)) | |
| } | |
| else -> return null | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: detekt
[warning] 136-136: Restrict the number of return statements in methods.
Function basicAuthenticationCredentials has 5 return statements which exceeds the limit of 2.
[warning] 146-146: Caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
Caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
🤖 Prompt for AI Agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/BasicAuth.kt` around lines
136 - 158, The basicAuthenticationCredentials function ignores the charset
parameter and always decodes bytes as UTF-8; update it to use the provided
charset (or ISO-8859-1 when charset == null for legacy RFC 7617 behavior) when
converting decoded Base64 bytes to String, and remove the redundant "0 +
bytes.size" usage—i.e., determine val effectiveCharset = charset ?:
Charsets.ISO_8859_1 and create the userPass string from the bytes using that
charset before finding the colon and returning UserPasswordCredential in
basicAuthenticationCredentials (keep the existing parseAuthorizationHeader /
HttpAuthHeader.Single checks and error handling).
| public fun HttpAuthHeader.Parameterized.toDigestCredential(): DigestCredential = DigestCredential( | ||
| parameter("realm")!!, | ||
| parameter("username")!!, | ||
| parameter("uri")!!, | ||
| parameter("nonce")!!, | ||
| parameter("opaque"), | ||
| parameter("nc"), | ||
| parameter("algorithm"), | ||
| parameter("response")!!, | ||
| parameter("cnonce"), | ||
| parameter("qop") | ||
| ) |
There was a problem hiding this comment.
!! operators on header parameters risk NPE on malformed Digest headers.
If a client sends a Digest authorization header missing any of realm, username, uri, nonce, or response, this function will throw a NullPointerException instead of gracefully returning null. The caller in onAuthenticate (Line 48-54) would then propagate the crash rather than treating it as missing credentials.
🐛 Proposed fix — return null on missing required parameters
public fun HttpAuthHeader.Parameterized.toDigestCredential(): DigestCredential? = DigestCredential(
- parameter("realm")!!,
- parameter("username")!!,
- parameter("uri")!!,
- parameter("nonce")!!,
+ parameter("realm") ?: return null,
+ parameter("username") ?: return null,
+ parameter("uri") ?: return null,
+ parameter("nonce") ?: return null,
parameter("opaque"),
parameter("nc"),
parameter("algorithm"),
- parameter("response")!!,
+ parameter("response") ?: return null,
parameter("cnonce"),
parameter("qop")
)This also requires updating the call site return type and toDigestCredential() usages (Lines 50 and 225) to handle the nullable return.
🤖 Prompt for AI Agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/DigestAuth.kt` around lines
239 - 250, The toDigestCredential function should stop using the non-null
asserted (!!) parameter() calls and instead validate required fields (realm,
username, uri, nonce, response) and return null if any are missing; change its
signature to return DigestCredential? and construct a DigestCredential only when
all required parameters are present, passing optional parameters as before.
Update call sites that invoke toDigestCredential (notably onAuthenticate and the
other usage around the existing toDigestCredential call) to handle a nullable
return by treating null as missing/invalid credentials and proceeding
accordingly. Ensure no other code assumes a non-null result from
toDigestCredential and add null checks or early returns where needed.
| fun timeMillis(): Float { | ||
| return System.nanoTime() / 1000_000f | ||
| } |
There was a problem hiding this comment.
timeMillis() uses Float, causing catastrophic cancellation with System.nanoTime().
System.nanoTime() returns a large Long (often 13+ digits). Dividing by 1_000_000f produces a Float with only ~7 significant digits, so the absolute millisecond value is heavily rounded. When the drip function computes timeMillis() - start (line 402), the subtraction of two similarly-rounded large values can yield zero or wildly inaccurate deltas.
Use Double or compute in Long millis directly.
🐛 Fix precision
- fun timeMillis(): Float {
- return System.nanoTime() / 1000_000f
+ fun timeMillis(): Float {
+ return System.nanoTime() / 1_000_000.0.toFloat()
}This still suffers from the same Float issue. Better approach:
- fun timeMillis(): Float {
- return System.nanoTime() / 1000_000f
+ fun timeMillis(): Double {
+ return System.nanoTime() / 1_000_000.0
}This requires changing the drip function signature to accept () -> Double and updating internal arithmetic accordingly.
🤖 Prompt for AI Agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Dynamic.kt` around lines 153
- 155, timeMillis() currently returns Float causing precision loss; change it to
return a higher-precision type (preferably Double or Long) and update any
callers accordingly: modify timeMillis() to compute milliseconds as either
System.nanoTime() / 1_000_000.0 and return Double, or System.nanoTime() /
1_000_000L and return Long, then update drip (and its parameter type if it
accepts a time supplier) to accept the same type (() -> Double or () -> Long)
and adjust the subtraction/interval arithmetic in drip to use Double/Long
arithmetic; ensure all usages of timeMillis() and drip use the new signature and
types.
| when (val port = request.local.serverPort) { | ||
| 80 if request.local.scheme == "http" -> {} | ||
| 443 if request.local.scheme == "https" -> {} | ||
| 0 -> {} // Test engine |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Kotlin version and language settings in build files
fd -e kts -e gradle -e properties | xargs grep -i -E 'kotlin|language.?version|api.?version' 2>/dev/null | head -40Repository: ktorio/ktor-samples
Length of output: 3094
🏁 Script executed:
#!/bin/bash
# Find httpbin build files
fd -path "*/httpbin/*" -e "gradle.kts" -o -path "*/httpbin/*" -e "properties"Repository: ktorio/ktor-samples
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Also search for any languageVersion or apiVersion settings specifically
rg "languageVersion|apiVersion" --type gradle --type properties -A 2 -B 2Repository: ktorio/ktor-samples
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Find httpbin build files
find . -path "*httpbin*" -name "*.gradle.kts" -o -path "*httpbin*" -name "*.gradle" -o -path "*httpbin*" -name "gradle.properties"Repository: ktorio/ktor-samples
Length of output: 147
🏁 Script executed:
#!/bin/bash
# Search for languageVersion or apiVersion in all build files
rg "languageVersion|apiVersion" -A 2 -B 2Repository: ktorio/ktor-samples
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check httpbin/build.gradle.kts
cat httpbin/build.gradle.ktsRepository: ktorio/ktor-samples
Length of output: 2021
🏁 Script executed:
#!/bin/bash
# Check httpbin/gradle.properties
cat httpbin/gradle.propertiesRepository: ktorio/ktor-samples
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Read the specific lines mentioned in the review (lines 245-248)
sed -n '240,255p' httpbin/src/main/kotlin/io/ktor/samples/httpbin/Responses.ktRepository: ktorio/ktor-samples
Length of output: 573
🌐 Web query:
Kotlin 2.1 guard conditions when statement feature
💡 Result:
Kotlin 2.1.0 (released Nov 27, 2024) added guard conditions for when with a subject as a preview/opt-in language feature. Guard conditions let you add an extra boolean check to a when branch by writing if ... after the primary match. [1]
Syntax
when (animal) {
is Animal.Dog -> feedDog()
is Animal.Cat if !animal.mouseHunter -> feedCat()
else -> println("Unknown")
}A guarded branch runs only if both the primary condition and the guard are true; if the primary condition doesn’t match, the guard isn’t evaluated. You can also use else if, and combine multiple guard checks with && / ||. [2]
Key limitations / rules
- Works only for
whenthat has a subject (when (x) { ... }). [1] - You can’t combine a guard with a comma-separated branch like
0, 1 -> .... [2] whenexpressions must still be exhaustive (guards don’t remove that requirement). [2]
How to enable (preview)
Pass the compiler flag:
- CLI:
-Xwhen-guards - Gradle (Kotlin DSL):
kotlin {
compilerOptions {
freeCompilerArgs.add("-Xwhen-guards")
}
}[1]
Sources: Kotlin “What’s new in 2.1.0” [1], Kotlin control-flow docs (when guard conditions) [2].
🏁 Script executed:
#!/bin/bash
# Search for compilerOptions or freeCompilerArgs in httpbin build files
rg "compilerOptions|freeCompilerArgs|Xwhen" httpbin/Repository: ktorio/ktor-samples
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Also check if there's a kotlin block with compiler settings
sed -n '/^kotlin {/,/^}/p' httpbin/build.gradle.ktsRepository: ktorio/ktor-samples
Length of output: 94
Add the -Xwhen-guards compiler flag to enable guard conditions.
Guard conditions in when branches (syntax like 80 if condition ->) are a Kotlin 2.1+ preview feature that requires explicit opt-in via the -Xwhen-guards compiler flag. The project uses Kotlin 2.3.0, but this code will not compile without the flag enabled.
Add to httpbin/build.gradle.kts:
kotlin {
jvmToolchain(17)
compilerOptions {
freeCompilerArgs.add("-Xwhen-guards")
}
}🤖 Prompt for AI Agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Responses.kt` around lines
245 - 248, The Kotlin code uses guarded when branches (the expression starting
with "when (val port = request.local.serverPort)" that contains branches like
"80 if request.local.scheme == \"http\" ->") which require the -Xwhen-guards
compiler flag; to fix, add the flag to the Kotlin compiler options in the module
build configuration (httpbin/build.gradle.kts) by adding "-Xwhen-guards" to
kotlin.compilerOptions.freeCompilerArgs so the guarded when syntax compiles.
| private fun virtualTimeApplication( | ||
| block: suspend ApplicationTestBuilder.( | ||
| scheduler: TestCoroutineScheduler | ||
| ) -> Unit | ||
| ) { | ||
| val scheduler = TestCoroutineScheduler() | ||
| val disp = StandardTestDispatcher(scheduler) | ||
|
|
||
| runTest(disp) { | ||
| runTestApplication(coroutineContext) { | ||
| engine { | ||
| dispatcher = disp | ||
| } | ||
|
|
||
| block(scheduler) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for runTestApplication definition
rg -n 'fun runTestApplication' --type=ktRepository: ktorio/ktor-samples
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Search for runTestApplication definition
rg -n 'fun runTestApplication'
# Also search for where it's defined or imported
rg -n 'runTestApplication' | head -50Repository: ktorio/ktor-samples
Length of output: 184
🏁 Script executed:
#!/bin/bash
# Read the imports and top of the test file
head -50 httpbin/src/test/kotlin/io/ktor/samples/httpbin/DynamicDataTest.kt
# Search for similar function names
rg -n 'fun.*Application.*\(' | grep -i test | head -20
# Search for testApplication which is a standard Ktor API
rg -n 'testApplication'Repository: ktorio/ktor-samples
Length of output: 50376
🏁 Script executed:
#!/bin/bash
# Search in .kt files for runTestApplication definition or usage
find . -name "*.kt" -type f | xargs grep -l "runTestApplication" | head -20
# Look at the file and see what's being imported and used
echo "=== Checking imports and usage in DynamicDataTest.kt ==="
sed -n '1,50p' httpbin/src/test/kotlin/io/ktor/samples/httpbin/DynamicDataTest.kt
# Search for testApplication (the standard Ktor API)
echo "=== Searching for testApplication ==="
grep -n "testApplication" httpbin/src/test/kotlin/io/ktor/samples/httpbin/DynamicDataTest.kt | head -10
# Check if runTestApplication is a custom function in the file
echo "=== Searching for fun runTest or similar patterns ==="
grep -n "fun run" httpbin/src/test/kotlin/io/ktor/samples/httpbin/DynamicDataTest.ktRepository: ktorio/ktor-samples
Length of output: 2477
🏁 Script executed:
#!/bin/bash
# Search all .kt files for runTestApplication definition
echo "=== Searching entire repo for 'runTestApplication' ==="
find . -type f -name "*.kt" -exec grep -H "runTestApplication" {} \;
# Also search in the common test utilities or base classes
echo ""
echo "=== Checking if there's a common test file ==="
find . -path "*test*" -name "*Test*.kt" -o -path "*test*" -name "*Base*.kt" | head -10Repository: ktorio/ktor-samples
Length of output: 970
🏁 Script executed:
#!/bin/bash
# Check other test files to see if they define or use any custom test utilities
echo "=== Checking if other test files in httpbin define custom utilities ==="
head -100 ./httpbin/src/test/kotlin/io/ktor/samples/httpbin/AnythingTest.kt | grep -E "fun |import"
# Look at the full context around line 554-571 to understand the code better
echo ""
echo "=== Full context of the virtualTimeApplication function ==="
sed -n '554,571p' ./httpbin/src/test/kotlin/io/ktor/samples/httpbin/DynamicDataTest.ktRepository: ktorio/ktor-samples
Length of output: 1309
runTestApplication is undefined and will cause a compilation error.
runTestApplication does not exist in the standard Ktor testing API or anywhere in this codebase. The available testing function from io.ktor.server.testing is testApplication. This call will fail to compile unless this function is defined elsewhere in the codebase.
🤖 Prompt for AI Agents
In `@httpbin/src/test/kotlin/io/ktor/samples/httpbin/DynamicDataTest.kt` around
lines 554 - 571, The call to the non-existent runTestApplication causes a
compile error; replace it with the Ktor testing helper testApplication and keep
the virtual-time setup: inside runTest(disp) call testApplication { engine {
dispatcher = disp } and then invoke the existing block(scheduler) within that
lambda. Update references to runTestApplication -> testApplication and ensure
you still pass the dispatcher into engine and call block(scheduler) from the
ApplicationTestBuilder context.
| private fun isValidUtf8(payload: ByteArray): Boolean { | ||
| val decoder = Charsets.UTF_8.newDecoder() | ||
| .onMalformedInput(CodingErrorAction.REPORT) | ||
| .onUnmappableCharacter(CodingErrorAction.REPORT) | ||
|
|
||
| return try { | ||
| decoder.decode(ByteBuffer.wrap(payload)) | ||
| true | ||
| } catch (_: CharacterCodingException) { | ||
| false | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the import statements in the file and look for CharacterCodingException
echo "=== Checking for CharacterCodingException import/usage ==="
rg -n 'CharacterCodingException|import java.nio' httpbin/src/test/kotlin/io/ktor/samples/httpbin/ResponseFormatTest.kt
echo -e "\n=== Checking all imports in the file ==="
rg -n '^import' httpbin/src/test/kotlin/io/ktor/samples/httpbin/ResponseFormatTest.kt | head -30Repository: ktorio/ktor-samples
Length of output: 1256
🏁 Script executed:
#!/bin/bash
# Check for any wildcard imports that might cover CharacterCodingException
echo "=== Checking for wildcard imports ==="
rg -n 'import.*\*' httpbin/src/test/kotlin/io/ktor/samples/httpbin/ResponseFormatTest.kt
echo -e "\n=== Full context of lines 195-205 ==="
sed -n '195,205p' httpbin/src/test/kotlin/io/ktor/samples/httpbin/ResponseFormatTest.ktRepository: ktorio/ktor-samples
Length of output: 315
Add missing import for CharacterCodingException.
The code uses CharacterCodingException in the catch block (line 199) but does not import it. This will cause a compilation error.
Add to the import block:
import java.nio.charset.CharacterCodingException🤖 Prompt for AI Agents
In `@httpbin/src/test/kotlin/io/ktor/samples/httpbin/ResponseFormatTest.kt` around
lines 191 - 202, The isValidUtf8 function catches CharacterCodingException but
the class isn't imported; add the missing import for
java.nio.charset.CharacterCodingException to the file so the catch clause
compiles and the isValidUtf8(ByteArray) function resolves correctly.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Dynamic.kt`:
- Around line 240-255: The route("/range/{numbytes}") currently uses
(call.parameters["numbytes"]?.toIntOrNull() ?: 0).coerceIn(0, 102400) which
silently clamps values >102400; change this to parse and explicitly validate the
number (e.g. toIntOrNull and check >0 and <=102400) and return
HttpStatusCode.BadRequest when out of range, and move the
call.response.headers.append(HttpHeaders.ETag, ...) and AcceptRanges header
writes to after successful validation so error responses do not include an ETag;
update the error message to match the validation logic and ensure the handler
returns after responding.
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/images.kt`:
- Around line 114-126: Fix the typo in the describe block for the "Images"
endpoint: update the summary string in the describe { ... } closure (the summary
property next to tag("Images")) from "Returns a simple image of the type suggest
by the Accept header." to "Returns a simple image of the type suggested by the
Accept header."
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Server.kt`:
- Around line 127-149: The digestProvider lambda uses
MessageDigest.getInstance(algorithm) with algorithm sourced from
parameters["algorithm"], which can throw NoSuchAlgorithmException for invalid
input; update the digestProvider in the digest("digest") block to catch
NoSuchAlgorithmException around MessageDigest.getInstance(algorithm) and handle
it by either falling back to a safe algorithm (e.g., "MD5") or returning null so
the request results in a 400/authorization failure rather than an unhandled 500;
ensure the change is applied within the digestProvider lambda where
MessageDigest.getInstance is called and preserve the existing return behavior
for valid inputs.
- Around line 122-126: The authenticate callback inside the bearer("bearer")
block should return a Principal instance rather than a raw String; update the
authenticate lambda (the authenticate { credentials -> ... } in Server.kt) to
return a Principal on success (for example instantiate UserIdPrincipal with
credentials.token or your own Principal implementation) and return null when
authentication fails so Ktor receives a proper Principal object.
🧹 Nitpick comments (4)
httpbin/src/main/kotlin/io/ktor/samples/httpbin/images.kt (2)
21-81: Content negotiation implementation usingparseHeaderValueis solid.The rewrite properly leverages Ktor's
parseHeaderValueto handle quality parameters, specificity ranking, and ordering — this addresses the original concern about manual Accept header parsing.Two minor observations:
- Inconsistent
Doublecomparison: Line 47 correctly uses an epsilon for floating-point quality comparison, but line 64 uses!=directly. Consider using the same epsilon-based check for consistency:- a!!.quality != b!!.quality -> a.quality.compareTo(b.quality) + abs(a!!.quality - b!!.quality) >= epsilon -> a.quality.compareTo(b.quality)(This would require hoisting
epsilonout of the inner loop or passing it as a parameter.)
- Unreachable null check: Lines 76–78 are dead code —
chosenfrom line 61–68 is filtered withe != null, soacceptcan never be null whenchosenis non-null.
100-113:whenstatement is not exhaustive — silent no-op on unhandled content types.If
imageContentTypesis later extended (e.g., withimage/avif), thiswhenblock will silently skip it because it's a statement, not an expression. Consider adding anelsebranch that falls back to PNG or throws, to prevent silent bugs.Proposed fix
ContentType.Image.PNG, ContentType.Image.Any -> { call.respondResource("sample.png") } + else -> { + call.respondResource("sample.png") + } }httpbin/src/main/kotlin/io/ktor/samples/httpbin/Server.kt (1)
202-208:Headers.toSortedMap()collapses multi-valued headers into a single comma-joined string.This is correct per HTTP semantics (RFC 7230 §3.2.2), but note that
Set-Cookieheaders should not be combined this way. For httpbin response inspection purposes this is fine, but worth noting if this utility is used more broadly.httpbin/src/main/kotlin/io/ktor/samples/httpbin/Dynamic.kt (1)
331-357:/stream/{n}cap of 102,400 JSON objects is very generous — consider a more practical limit.Each streamed JSON object includes headers, origin, URL, and args — easily hundreds of bytes each. At 102,400 objects, a single request could generate tens of megabytes. The original httpbin caps this at 100. While this was partially noted before, the current bound still seems excessively high for a JSON-streaming endpoint.
| route("/range/{numbytes}") { | ||
| install(PartialContent) | ||
| get { | ||
| val number = (call.parameters["numbytes"]?.toIntOrNull() ?: 0).coerceIn(0, 102400) | ||
|
|
||
| call.response.headers.append(HttpHeaders.ETag, "range$number") | ||
| call.response.headers.append(HttpHeaders.AcceptRanges, "bytes") | ||
|
|
||
| if (number == 0) { | ||
| call.respondText( | ||
| "number of bytes must be in the range (0, 102400]", | ||
| status = HttpStatusCode.BadRequest, | ||
| contentType = ContentType.Text.Html | ||
| ) | ||
| return@get | ||
| } |
There was a problem hiding this comment.
/range/{numbytes} silently clamps values above 102400 instead of rejecting them.
Line 243 uses coerceIn(0, 102400), so a request like /range/999999 silently becomes /range/102400 and succeeds. Meanwhile, line 250 has an error message stating the range is (0, 102400]. Consider rejecting out-of-range values explicitly for consistent behavior:
Proposed fix
- val number = (call.parameters["numbytes"]?.toIntOrNull() ?: 0).coerceIn(0, 102400)
+ val rawNumber = call.parameters["numbytes"]?.toIntOrNull() ?: 0
call.response.headers.append(HttpHeaders.ETag, "range$number")
call.response.headers.append(HttpHeaders.AcceptRanges, "bytes")
- if (number == 0) {
+ if (rawNumber !in 1..102400) {
call.respondText(
"number of bytes must be in the range (0, 102400]",
status = HttpStatusCode.BadRequest,
contentType = ContentType.Text.Html
)
return@get
}
+
+ val number = rawNumberNote: The ETag header is currently set before validation (line 245), meaning error responses also include an ETag (range0 or range102400). Moving the header append after validation would be cleaner.
🤖 Prompt for AI Agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Dynamic.kt` around lines 240
- 255, The route("/range/{numbytes}") currently uses
(call.parameters["numbytes"]?.toIntOrNull() ?: 0).coerceIn(0, 102400) which
silently clamps values >102400; change this to parse and explicitly validate the
number (e.g. toIntOrNull and check >0 and <=102400) and return
HttpStatusCode.BadRequest when out of range, and move the
call.response.headers.append(HttpHeaders.ETag, ...) and AcceptRanges header
writes to after successful validation so error responses do not include an ETag;
update the error message to match the validation logic and ensure the handler
returns after responding.
| }.describe { | ||
| tag("Images") | ||
|
|
||
| summary = "Returns a simple image of the type suggest by the Accept header." | ||
| responses { | ||
| HttpStatusCode.OK { | ||
| description = "An image." | ||
| for (contentType in imageContentTypes) { | ||
| contentType() | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Typo: "suggest" → "suggested".
Line 117: "Returns a simple image of the type suggest by the Accept header." should read "suggested".
Fix
- summary = "Returns a simple image of the type suggest by the Accept header."
+ summary = "Returns a simple image of the type suggested by the Accept header."📝 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.
| }.describe { | |
| tag("Images") | |
| summary = "Returns a simple image of the type suggest by the Accept header." | |
| responses { | |
| HttpStatusCode.OK { | |
| description = "An image." | |
| for (contentType in imageContentTypes) { | |
| contentType() | |
| } | |
| } | |
| } | |
| } | |
| }.describe { | |
| tag("Images") | |
| summary = "Returns a simple image of the type suggested by the Accept header." | |
| responses { | |
| HttpStatusCode.OK { | |
| description = "An image." | |
| for (contentType in imageContentTypes) { | |
| contentType() | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/images.kt` around lines 114 -
126, Fix the typo in the describe block for the "Images" endpoint: update the
summary string in the describe { ... } closure (the summary property next to
tag("Images")) from "Returns a simple image of the type suggest by the Accept
header." to "Returns a simple image of the type suggested by the Accept header."
| bearer("bearer") { | ||
| authenticate { credentials -> | ||
| credentials.token | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ktor bearer authentication authenticate callback return type Principal
💡 Result:
In Ktor Server Bearer auth, the authenticate { ... } callback you configure has this signature/return type:
suspend ApplicationCall.(BearerTokenCredential) -> Any?(this is the genericAuthenticationFunction<C>typealias) — i.e. it returnsAny?. Typically you return a Principal instance (for exampleUserIdPrincipal(...)) on success, ornullto indicate authentication failed. [1][2]
Example (from Ktor docs):
install(Authentication) {
bearer("auth-bearer") {
authenticate { tokenCredential ->
if (tokenCredential.token == "abc123") UserIdPrincipal("jetbrains") else null
}
}
}Returning null means “invalid/unauthorized token”. [3]
Sources: [1] [2] [3]
Return a Principal instance from authenticate, not a raw String.
Ktor's authenticate callback accepts Any?, so credentials.token will compile. However, the idiomatic pattern is to return a Principal subclass (e.g., UserIdPrincipal(credentials.token)) on successful authentication, or null if authentication fails.
🤖 Prompt for AI Agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Server.kt` around lines 122 -
126, The authenticate callback inside the bearer("bearer") block should return a
Principal instance rather than a raw String; update the authenticate lambda (the
authenticate { credentials -> ... } in Server.kt) to return a Principal on
success (for example instantiate UserIdPrincipal with credentials.token or your
own Principal implementation) and return null when authentication fails so Ktor
receives a proper Principal object.
| digest("digest") { | ||
| realm = "Fake Realm" | ||
| getAlgorithm = { | ||
| parameters["algorithm"] ?: "MD5" | ||
| } | ||
|
|
||
| digestProvider { _, realm -> | ||
| val user = parameters["user"] ?: return@digestProvider null | ||
| val password = parameters["password"] ?: return@digestProvider null | ||
| val algorithm = parameters["algorithm"] ?: "MD5" | ||
|
|
||
| MessageDigest.getInstance(algorithm).digest( | ||
| "$user:$realm:$password".toByteArray() | ||
| ) | ||
| } | ||
| validate { credentials -> | ||
| if (credentials.userName.isNotEmpty()) { | ||
| UserIdPrincipal(credentials.userName) | ||
| } else { | ||
| null | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
MessageDigest.getInstance(algorithm) can throw NoSuchAlgorithmException if an unsupported algorithm name is provided via route parameters.
Since algorithm comes from user-controlled parameters["algorithm"], a request like /digest-auth/SHA-999/user/pass would cause an unhandled exception (likely a 500). Consider catching NoSuchAlgorithmException and returning a 400 or falling back to MD5.
🤖 Prompt for AI Agents
In `@httpbin/src/main/kotlin/io/ktor/samples/httpbin/Server.kt` around lines 127 -
149, The digestProvider lambda uses MessageDigest.getInstance(algorithm) with
algorithm sourced from parameters["algorithm"], which can throw
NoSuchAlgorithmException for invalid input; update the digestProvider in the
digest("digest") block to catch NoSuchAlgorithmException around
MessageDigest.getInstance(algorithm) and handle it by either falling back to a
safe algorithm (e.g., "MD5") or returning null so the request results in a
400/authorization failure rather than an unhandled 500; ensure the change is
applied within the digestProvider lambda where MessageDigest.getInstance is
called and preserve the existing return behavior for valid inputs.
bjhham
left a comment
bjhham
left a comment
There was a problem hiding this comment.
Github might be a little broken from the size, but it would be nice to have a little more details about the httpbin project in the README.
Very cool sample project, it's got everything!
| """ | ||
| <!DOCTYPE html> | ||
| <title>404 Not Found</title> | ||
| <h1>Not Found</h1> | ||
| <p>The requested URL was not found on the server. | ||
| If you entered the URL manually please check your spelling and try again.</p> | ||
| """.trimIndent() |
There was a problem hiding this comment.
Browsers might handle this ok, but it's probably better to return valid HTML - it's missing the html, head and body elements. Would be more tidy to use the HTML DSL or just extract a function or string template.
| openApi { | ||
| enabled = true | ||
| codeInferenceEnabled = false | ||
| } |
There was a problem hiding this comment.
Since you're not using the comment processing or the code inference anywhere, you can remove this.
| .-''''''-. | ||
| .' _ _ '. | ||
| / O O \\ | ||
| : : | ||
| | | | ||
| : __ : | ||
| \ .-"` `"-. / | ||
| '. .' | ||
| '-......-' | ||
| YOU SHOULDN'T BE HERE | ||
| """, contentType = ContentType.Text.Plain) |
There was a problem hiding this comment.
Your face is hanging a bit to the left here, needs some indent.
| * | ||
| * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.auth.DigestAuthenticationProvider.Config.algorithmName) | ||
| */ | ||
| // public var algorithmName: String = "MD5" |
There was a problem hiding this comment.
Left in some commented out code here.
| * @property realm specifies the value to be passed in the `WWW-Authenticate` header. | ||
| * @property algorithmName a message digest algorithm to be used. Usually only `MD5` is supported by clients. | ||
| */ | ||
| class DigestAuthenticationProvider internal constructor( |
There was a problem hiding this comment.
I take it the plan here is to replace this after the release of ktorio/ktor#5348 ?
2 participants
https://youtrack.jetbrains.com/issue/KTOR-822