Skip to content

Commit 050f6ea

Browse files
authored
[Ktor] Use OkHttp logging format (#2631)
* Extract logging plugin into separate method, use OkHttp format * Update sensitive header redaction checks in HttpClientBuilderTest
1 parent 68e1c93 commit 050f6ea

2 files changed

Lines changed: 38 additions & 29 deletions

File tree

core/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import io.mockk.impl.annotations.MockK
3535
import io.mockk.junit4.MockKRule
3636
import kotlinx.coroutines.test.runTest
3737
import org.junit.Assert.assertEquals
38-
import org.junit.Assert.assertFalse
3938
import org.junit.Assert.assertNull
4039
import org.junit.Assert.assertTrue
4140
import org.junit.Before
@@ -296,11 +295,9 @@ class HttpClientBuilderTest {
296295
}
297296

298297
val log = messages.joinToString("\n")
299-
// sensitive header values must not be logged
300-
assertFalse(log.contains("super-secret-token-12345"))
301-
assertFalse(log.contains("secret-cookie-value"))
302-
// they must be redacted
303-
assertTrue(log.contains("***"))
298+
// sensitive header values must be redacted
299+
assertTrue(log.contains("Authorization: ██\n"))
300+
assertTrue(log.contains("Set-Cookie: ██\n"))
304301
}
305302

306303

core/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import io.ktor.client.plugins.contentnegotiation.ContentNegotiation
3333
import io.ktor.client.plugins.cookies.HttpCookies
3434
import io.ktor.client.plugins.logging.LogLevel
3535
import io.ktor.client.plugins.logging.Logging
36+
import io.ktor.client.plugins.logging.LoggingFormat
3637
import io.ktor.http.HttpHeaders
3738
import io.ktor.http.URLBuilder
3839
import io.ktor.http.URLProtocol
@@ -356,6 +357,38 @@ class HttpClientBuilder private constructor(
356357
}
357358
}
358359

360+
private fun HttpClientConfig<*>.installLoggingPlugin() {
361+
install(Logging) {
362+
// log to configured java.util.Logger with "finest"
363+
logger = object : io.ktor.client.plugins.logging.Logger {
364+
override fun log(message: String) {
365+
config.logger.finest(message)
366+
}
367+
}
368+
369+
// use configured log level (with/without body)
370+
level = config.trafficLogLevel
371+
372+
// LoggingFormat.Default shows multiple list headers with ";"
373+
// https://github.com/bitfireAT/davx5-ose/issues/2630 / https://youtrack.jetbrains.com/issue/KTOR-8385
374+
format = LoggingFormat.OkHttp
375+
376+
// don't log some confidential headers
377+
val headersToIgnore = arrayOf(
378+
HttpHeaders.Authorization,
379+
HttpHeaders.Cookie,
380+
HttpHeaders.SetCookie,
381+
"Set-Cookie2" // obsoleted, but included here for good measure
382+
)
383+
sanitizeHeader { header ->
384+
headersToIgnore.any { headerToIgnore ->
385+
header.equals(headerToIgnore, ignoreCase = true)
386+
}
387+
}
388+
}
389+
390+
}
391+
359392
/**
360393
* Installs all Ktor-level plugins (cookies, timeouts, content negotiation, user agent,
361394
* compression, auth, logging) that are shared between [build] (real [OkHttp] engine) and
@@ -400,29 +433,8 @@ class HttpClientBuilder private constructor(
400433
ContentEncoding()
401434

402435
// add network logging (with redaction of sensitive headers), if requested
403-
if (config.logger.isLoggable(Level.FINEST)) {
404-
install(Logging) {
405-
logger = object : io.ktor.client.plugins.logging.Logger {
406-
override fun log(message: String) {
407-
config.logger.finest(message)
408-
}
409-
}
410-
level = config.trafficLogLevel
411-
412-
// don't log some confidential headers
413-
val headersToIgnore = arrayOf(
414-
HttpHeaders.Authorization,
415-
HttpHeaders.Cookie,
416-
HttpHeaders.SetCookie,
417-
"Set-Cookie2" // obsoleted, but included here for good measure
418-
)
419-
sanitizeHeader { header ->
420-
headersToIgnore.any { headerToIgnore ->
421-
header.equals(headerToIgnore, ignoreCase = true)
422-
}
423-
}
424-
}
425-
}
436+
if (config.logger.isLoggable(Level.FINEST))
437+
installLoggingPlugin()
426438
}
427439

428440
/**

0 commit comments

Comments
 (0)