Skip to content

Modernize String-based charset APIs to java.nio.charset.Charset#4606

Merged
janhoy merged 5 commits into
apache:mainfrom
janhoy:charset-api-modernization
Jul 3, 2026
Merged

Modernize String-based charset APIs to java.nio.charset.Charset#4606
janhoy merged 5 commits into
apache:mainfrom
janhoy:charset-api-modernization

Conversation

@janhoy

@janhoy janhoy commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Split out of the Error Prone 2.50.0 upgrade (#4604) per review feedback there: the JdkObsolete pattern flags charset-name String overloads, and this modernization changes exception behavior, so it deserves its own review.

  • Replaces String charset-name APIs (new String(bytes, name), new InputStreamReader(is, name), URLEncoder.encode(s, name), ...) with their Charset overloads.
  • Charset.forName() throws unchecked IllegalArgumentException where the old APIs threw the checked UnsupportedEncodingException. Since the charset name often comes from a client-supplied Content-Type header, a new ContentStreamBase.charsetForName() helper converts it back to UnsupportedEncodingException so an invalid charset stays an I/O error instead of escaping as a runtime exception. HttpSolrClient catches IllegalArgumentException explicitly.
  • Removes now-dead UnsupportedEncodingException handling around compile-time-constant charsets (e.g. SolrParams.toQueryString).

#4604 will suppress the corresponding JdkObsolete findings with TODO comments pointing here; once this merges, that branch drops the suppressions on rebase.

Replace charset-name String overloads (which throw the checked
UnsupportedEncodingException) with Charset overloads, as flagged by
Error Prone's JdkObsolete pattern.

Charset.forName() throws unchecked IllegalArgumentException, so where
the charset name comes from a client-supplied Content-Type header, the
new ContentStreamBase.charsetForName() converts it back to
UnsupportedEncodingException, preserving the previous contract that an
invalid charset is handled as an I/O error. Dead
UnsupportedEncodingException handling for compile-time-constant
charsets is removed.
Comment thread changelog/unreleased/PR#4606-charset-api-modernization.yml
janhoy added a commit to janhoy/solr that referenced this pull request Jul 3, 2026
Per review feedback, behavior-changing fixes are split into their own PRs
so this PR only carries the mechanical Error Prone 2.50.0 compliance work:

- Reference-equality bug fixes (ZkMaintenanceUtils, HttpShardHandler,
  ZkShardTermsTest) moved to apache#4605
- Charset API modernization (JdkObsolete findings) moved to
  apache#4606

The affected sites revert to the code on main, with @SuppressWarnings and
a TODO comment pointing at the PR that carries the real fix; the
suppressions are dropped when those PRs merge and this branch rebases.
@janhoy janhoy requested review from dsmiley and epugh July 3, 2026 11:36
Comment thread solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java
Comment thread solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java Outdated
Comment thread changelog/unreleased/PR#4606-charset-api-modernization.yml Outdated
@janhoy janhoy requested a review from dsmiley July 3, 2026 15:10

@dsmiley dsmiley left a comment

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.

Thanks

@janhoy janhoy merged commit da4e044 into apache:main Jul 3, 2026
6 checks passed
@janhoy janhoy deleted the charset-api-modernization branch July 3, 2026 21:06
janhoy added a commit that referenced this pull request Jul 3, 2026
janhoy added a commit to janhoy/solr that referenced this pull request Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants