Skip to content

CLI: Decode stdin using client charset; add tests for description/display name#25921

Closed
shbhmexe wants to merge 4 commits intojenkinsci:masterfrom
shbhmexe:fix/cli-stdin-charset-encoding
Closed

CLI: Decode stdin using client charset; add tests for description/display name#25921
shbhmexe wants to merge 4 commits intojenkinsci:masterfrom
shbhmexe:fix/cli-stdin-charset-encoding

Conversation

@shbhmexe
Copy link
Contributor

This deliberately does not yet change the stdin sentinel conventions (SetBuildDescriptionCommand uses =, SetBuildDisplayNameCommand uses -). That alignment can be a follow‑up change. XML‑based CLI commands (create/update job/view/node) are intentionally untouched; they rely on the XML prolog/BOM for charset detection.

Testing done

  • Automated tests: two new unit tests prove stdin is decoded using the client charset (Windows‑1252 case).
  • Basic interactive tests: piping non‑ASCII files via CLI with explicit -Dfile.encoding works as expected.

Proposed changelog entries

  • Decode text read from stdin in CLI commands using the client charset negotiated by the CLI transport (fixes mojibake in mixed‑encoding scenarios). Affects: set-build-description, set-build-display-name, and groovy.

Proposed changelog category

/label bug

Proposed upgrade guidelines

No action is required for most users. If any scripts relied on the controller’s platform default decoding ignoring the client charset, be aware that stdin is now decoded using the client charset (consistent with CLI output). If needed, set the desired charset on the client JVM via -Dfile.encoding.

Submitter checklist

  • The issue is well‑described.
  • Changelog entry is in imperative mood and appropriate for users.
  • There is automated testing covering the change, or justification is provided.
  • No new public APIs; no new @SInCE required.
  • No new deprecations introduced.
  • UI/CSP unaffected.
  • Not a dependency update.

Desired reviewers

Before the changes are marked as ready-for-merge:

Maintainer checklist

  • There are at least two (2) approvals and no outstanding requests for change.
  • Conversations in the pull request are resolved, or it is explicit that a reviewer is not blocking the change.
  • Changelog entries are accurate and human‑readable, labels set correctly for automatic generation.
  • If upgrade steps are needed, the upgrade-guide-needed label is set and the Proposed upgrade guidelines section is present.
  • If backporting to LTS is sensible, mark as lts-candidate.

…play name

•  SetBuildDescriptionCommand: decode stdin via getClientCharset()
•  SetBuildDisplayNameCommand: decode stdin via getClientCharset()
•  GroovyCommand: decode '=' script from stdin using client charset

Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
@comment-ops-bot comment-ops-bot bot added the bug For changelog: Minor bug. Will be listed after features label Dec 11, 2025
@NotMyFault
Copy link
Member

Automated tests: two new unit tests prove stdin is decoded using the client charset (Windows‑1252 case).

There are no new unit tests.

@NotMyFault NotMyFault removed the bug For changelog: Minor bug. Will be listed after features label Jan 2, 2026
@NotMyFault
Copy link
Member

I'll go ahead and close the PR, given the description does not align with the change submitted and the submitter has abandoned the PR.

@NotMyFault NotMyFault closed this Jan 2, 2026
@comment-ops-bot comment-ops-bot bot added the bug For changelog: Minor bug. Will be listed after features label Jan 2, 2026
@NotMyFault NotMyFault added declined This PR doesn't match our acceptance criteria and removed bug For changelog: Minor bug. Will be listed after features labels Jan 2, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2026

Missing required label for changelog. Requires at least 1 of: bug, developer, dependencies, internal, localization, major-bug, major-rfe, rfe, regression-fix, removed, skip-changelog. Found: declined.

You can add the required label by adding a comment with the following text: /label <category>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

declined This PR doesn't match our acceptance criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants