Skip to content

🐛 bug: escape HTML output in Ctx.Format - #4232

Merged
ReneWerner87 merged 1 commit into
v2from
update-html-format-to-escape-content
Apr 25, 2026
Merged

🐛 bug: escape HTML output in Ctx.Format#4232
ReneWerner87 merged 1 commit into
v2from
update-html-format-to-escape-content

Conversation

@gaby

@gaby gaby commented Apr 25, 2026

Copy link
Copy Markdown
Member

Motivation

  • Prevent unescaped HTML injection when Ctx.Format renders HTML by escaping the formatted string before embedding it in the <p> wrapper while keeping JSON/TXT/XML branches RFC-consistent and unchanged.

Description

  • Import html and apply html.EscapeString(b) in the html branch of func (c *Ctx) Format(body interface{}) error so content is escaped before wrapping with <p>...</p> in ctx.go.
  • Add/adjust assertions in Test_Ctx_Format to verify that HTML mode escapes a <script> payload and that JSON/XML behavior remains unchanged by comparing Format output to direct c.JSON/c.XML output where appropriate in ctx_test.go.
  • Searched for a v3 equivalent (DefaultRes.AutoFormat / res.go) in this checkout and did not find a matching file or symbol, so no v3-side change was applied here.

@coderabbitai

coderabbitai Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 16100126-306e-470b-92e4-76c94137147e

📥 Commits

Reviewing files that changed from the base of the PR and between 6cba195 and ba5a98e.

📒 Files selected for processing (2)
  • ctx.go
  • ctx_test.go

Walkthrough

The Format method's HTML response path is updated to HTML-escape formatted string values before wrapping them in a paragraph element, preventing raw HTML character insertion. Tests are added to validate that <script> tags are properly escaped across HTML, JSON, and XML response types.

Changes

Cohort / File(s) Summary
HTML Escaping in Format Method
ctx.go
Added html import and replaced direct string concatenation with html.EscapeString(b) in the HTML response path to prevent raw HTML/script injection.
Test Coverage for Escaping
ctx_test.go
Extended Test_Ctx_Format suite with tests validating that <script> tags are properly escaped when Accept requests text/html, application/json, or application/xml. Includes cross-check for JSON consistency.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • sixcolors
  • efectn
  • ReneWerner87

Poem

🐰 A script tag tried to break free,
But escaping made it safe, you see!
With HTML guards and tests so true,
No injection tricks will get through! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description adequately explains the motivation and technical implementation, but does not follow the repository's PR template structure with required sections and checklists. Consider providing a description that follows the repository template, including explicit sections for type of change, checklist items (tests added, docs updated), and commit formatting guidance.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: fixing HTML escaping in Ctx.Format to prevent HTML injection attacks.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch update-html-format-to-escape-content

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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.

@ReneWerner87 ReneWerner87 added this to v3 Apr 25, 2026
@ReneWerner87 ReneWerner87 added this to the v3 milestone Apr 25, 2026
@gaby gaby moved this to In Progress in v3 Apr 25, 2026

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request enhances security by implementing HTML escaping in the Format method to prevent XSS vulnerabilities. Corresponding test cases were added for HTML, JSON, and XML formats to ensure correct handling of script tags. A review comment suggests refactoring a JSON test case to use a direct string assertion for better clarity and consistency with other tests.

Comment thread ctx_test.go
@ReneWerner87 ReneWerner87 removed the v3 label Apr 25, 2026
@ReneWerner87 ReneWerner87 removed this from v3 Apr 25, 2026
@ReneWerner87 ReneWerner87 removed this from the v3 milestone Apr 25, 2026
@gaby
gaby marked this pull request as ready for review April 25, 2026 12:45
@gaby
gaby requested a review from a team as a code owner April 25, 2026 12:45
@gaby
gaby requested review from Copilot, efectn and sixcolors April 25, 2026 12:45
@ReneWerner87 ReneWerner87 added this to the v2.52.13 milestone Apr 25, 2026
@ReneWerner87 ReneWerner87 added this to v3 Apr 25, 2026
@ReneWerner87 ReneWerner87 modified the milestones: v2.52.13, v3 Apr 25, 2026

Copilot AI 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.

Pull request overview

This PR hardens Ctx.Format against HTML injection by escaping HTML output before embedding it in the <p>...</p> wrapper, while keeping JSON/TXT/XML behavior unchanged.

Changes:

  • Escape HTML output in Ctx.Format using html.EscapeString before wrapping in <p>...</p>.
  • Extend Test_Ctx_Format to assert HTML escaping for a <script> payload and to ensure JSON/XML formatting output matches the direct c.JSON/c.XML behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
ctx.go Escapes HTML branch output in Ctx.Format before wrapping in <p>...</p>.
ctx_test.go Adds assertions verifying HTML escaping and unchanged JSON/XML behavior.

@ReneWerner87
ReneWerner87 merged commit 616de00 into v2 Apr 25, 2026
40 checks passed
@ReneWerner87
ReneWerner87 deleted the update-html-format-to-escape-content branch April 25, 2026 12:57
@github-project-automation github-project-automation Bot moved this to Done in v3 Apr 25, 2026
@ReneWerner87 ReneWerner87 removed the v3 label Apr 25, 2026
@ReneWerner87 ReneWerner87 removed this from v3 Apr 25, 2026
@ReneWerner87 ReneWerner87 modified the milestones: v3, v2.52.13 Apr 25, 2026
@ReneWerner87 ReneWerner87 added this to v3 Apr 25, 2026
@ReneWerner87 ReneWerner87 modified the milestones: v2.52.13, v3 Apr 25, 2026
@github-project-automation github-project-automation Bot moved this to Done in v3 Apr 25, 2026
@gaby gaby removed the v3 label Apr 29, 2026
@ReneWerner87 ReneWerner87 modified the milestones: v3, v3.3.0 May 22, 2026
@ReneWerner87 ReneWerner87 modified the milestones: v3.3.0, v3 May 22, 2026
@ReneWerner87 ReneWerner87 modified the milestones: v3, v3.4.0 Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants