Skip to content

KTOR-9355 Deprecate HttpHeaders.AcceptCharset#5411

Merged
osipxd merged 1 commit intomainfrom
osipxd/deprecate-accept-charset
Apr 15, 2026
Merged

KTOR-9355 Deprecate HttpHeaders.AcceptCharset#5411
osipxd merged 1 commit intomainfrom
osipxd/deprecate-accept-charset

Conversation

@osipxd
Copy link
Copy Markdown
Member

@osipxd osipxd commented Mar 2, 2026

Subsystem
ktor-http

Motivation
KTOR-9355 Deprecate HttpHeaders.AcceptCharset

@osipxd osipxd self-assigned this Mar 2, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5f010b and 0ffe0f9.

📒 Files selected for processing (9)
  • ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/HttpPlainText.kt
  • ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CharsetTest.kt
  • ktor-http/common/src/io/ktor/http/HttpHeaders.kt
  • ktor-server/ktor-server-core/common/src/io/ktor/server/request/ApplicationRequestProperties.kt
  • ktor-server/ktor-server-plugins/ktor-server-content-negotiation/common/test/ContentNegotiationTest.kt
  • ktor-shared/ktor-serialization/common/src/ContentConverter.kt
  • ktor-shared/ktor-serialization/ktor-serialization-jackson/jvm/test/ServerJacksonTest.kt
  • ktor-shared/ktor-serialization/ktor-serialization-jackson3/jvm/test/ServerJacksonTest.kt
  • ktor-shared/ktor-serialization/ktor-serialization-tests/jvm/src/AbstractServerSerializationTest.kt

Walkthrough

This PR deprecates the Accept-Charset HTTP header constant across the Ktor framework and adds @Suppress("DEPRECATION") annotations throughout the codebase to suppress deprecation warnings where the deprecated header is referenced.

Changes

Cohort / File(s) Summary
Header Deprecation
ktor-http/common/src/io/ktor/http/HttpHeaders.kt
Introduced @Deprecated annotation on AcceptCharset constant with RFC 9110 message; added @Suppress("DEPRECATION") to getAcceptCharset() function.
Client-Side Extensions
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/HttpPlainText.kt, ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/CharsetTest.kt
Added @Suppress("DEPRECATION") annotations to suppress warnings for deprecated Accept-Charset usage in plugin extension and test class.
Server-Side Extensions
ktor-server/ktor-server-core/common/src/io/ktor/server/request/ApplicationRequestProperties.kt, ktor-server/ktor-server-plugins/.../ContentNegotiationTest.kt
Added @Suppress("DEPRECATION") on acceptCharset extension functions and test methods to suppress deprecation warnings.
Serialization Framework and Tests
ktor-shared/ktor-serialization/common/src/ContentConverter.kt, ktor-shared/ktor-serialization/.../test/ServerJacksonTest.kt, ktor-shared/ktor-serialization/.../AbstractServerSerializationTest.kt
Added @Suppress("DEPRECATION") in serialization module and test files; refactored imports in ContentConverter.kt to use flow-based operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • bjhham
  • marychatte
  • zibet27
🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete. While it correctly identifies the subsystem and links to the motivation issue KTOR-9355, the required 'Solution' section is entirely missing. Add a 'Solution' section explaining how HttpHeaders.AcceptCharset is deprecated and what the rationale is for the deprecation.
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: deprecating the HttpHeaders.AcceptCharset header, which aligns with the changeset's primary objective.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch osipxd/deprecate-accept-charset

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.

* [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.request.acceptCharsetItems)
*/
@Suppress("DEPRECATION")
public fun ApplicationRequest.acceptCharsetItems(): List<HeaderValue> =
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Should we also deprecate these functions? My thought was that we should discourage users from adding this header, but it's okay to try to read it, because server is usually out of control.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think reading it in is fine, maybe exclude this one.

@osipxd osipxd requested a review from bjhham March 18, 2026 15:54
Copy link
Copy Markdown
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.

Could you include some KDoc or a deprecation message that either references or summarizes the reason in the RFC note?

* [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.request.acceptCharsetItems)
*/
@Suppress("DEPRECATION")
public fun ApplicationRequest.acceptCharsetItems(): List<HeaderValue> =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think reading it in is fine, maybe exclude this one.

Comment on lines +21 to 25
@Deprecated(
"The Accept-Charset request header has been deprecated in RFC 9110. " +
"UTF-8 should be used by default in modern applications."
)
public const val AcceptCharset: String = "Accept-Charset"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@bjhham, could you check if this deprecation message is enough?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm 👍

@osipxd osipxd merged commit 8c1eb0d into main Apr 15, 2026
25 of 27 checks passed
@osipxd osipxd deleted the osipxd/deprecate-accept-charset branch April 15, 2026 11:42
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.

2 participants