Skip to content

Conversation

@zibet27
Copy link
Collaborator

@zibet27 zibet27 commented Jan 26, 2026

Subsystem
Shared

Motivation
There are some usages of the deprecated String.encodeBase64 method that lead to warnings during tests and create noise.

@zibet27 zibet27 requested review from bjhham and osipxd January 26, 2026 15:55
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 26, 2026

Walkthrough

Replaced usages of the custom encodeBase64() extension with kotlin.io.encoding.Base64.encode() across client, server, HTTP, websocket, and test modules; adjusted imports accordingly. Also removed a test-server makeString helper and dropped an internal logger in OAuth2 implementation.

Changes

Cohort / File(s) Summary
Client auth & request tests
ktor-client/ktor-client-apache/jvm/test/.../RequestProducerTest.kt, ktor-client/ktor-client-core/common/src/io/ktor/client/request/utils.kt, ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BasicAuthProvider.kt, ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/Generators.kt
Replaced encodeBase64() uses with Base64.encode(...); added kotlin.io.encoding.Base64 imports and removed io.ktor.util.* imports; logic unchanged.
HTTP & WebSocket
ktor-http/common/src/io/ktor/http/Cookie.kt, ktor-http/common/src/io/ktor/http/websocket/Utils.kt
Switched BASE64 encoding paths to Base64.encode(...) for cookie and websocket nonce handling; updated imports.
Server auth implementation & tests
ktor-server/ktor-server-plugins/ktor-server-auth/common/src/io/ktor/server/auth/OAuth2.kt, ktor-server/ktor-server-plugins/ktor-server-auth/common/test/.../AuthBuildersTest.kt, .../BasicAuthTest.kt, .../CryptoTest.kt, .../OAuth2Test.kt
Replaced encodeBase64() with Base64.encode(...) in auth header construction and tests; removed an internal logger import/instance from OAuth2.
Server sessions & test utilities
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/sessions/SessionTest.kt, ktor-test-server/src/main/kotlin/test/server/ServerUtils.kt
Session cookie encoding switched to Base64.encode(...); removed makeString(size: Int) helper and its io.ktor.util.* import.
Base64 tests
ktor-utils/common/test/io/ktor/util/Base64Test.kt
Adjusted test encodings to use byte-array based encoding paths and added deprecation suppression where applicable.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • osipxd
  • bjhham
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.34% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description provides subsystem and motivation but omits the required Solution section describing how the deprecated method was replaced. Add a Solution section explaining that String.encodeBase64 was replaced with kotlin.io.encoding.Base64.encode() calls throughout the codebase.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing deprecated String.encodeBase64 usage from the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@bjhham bjhham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 🙏

Please note, you've have angered the linter

@zibet27 zibet27 enabled auto-merge January 27, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants