Skip to content

fix: append router revision link to HTML footers#30

Closed
Ductrung03 wants to merge 1 commit into
ubiquity:mainfrom
Ductrung03:fix/footer-revision-hash
Closed

fix: append router revision link to HTML footers#30
Ductrung03 wants to merge 1 commit into
ubiquity:mainfrom
Ductrung03:fix/footer-revision-hash

Conversation

@Ductrung03

Copy link
Copy Markdown

Summary

  • Inject router revision links into proxied HTML footers.
  • Preserve non-HTML passthrough behavior.
  • Remove stale Content-Length after HTML rewrites.

Verification

  • bun run test
  • bun run build
    Note: bun run type-check could not run locally because tsc was not available in this clone.
    B. Đợi maintainer duyệt

@ubiquity-os

ubiquity-os Bot commented Jun 8, 2026

Copy link
Copy Markdown

Warning

@Ductrung03 this pull request must be linked to an open issue and target the repository's default branch, or it will get automatically closed.

@ubiquity-os ubiquity-os Bot closed this Jun 8, 2026
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds git revision footer injection to HTML responses in the proxy worker. A new ROUTER_REPOSITORY_URL constant and withFooterRevision function conditionally rewrite proxied HTML responses by injecting or updating a git-revision footer link, escaping content safely and removing content-length when the body is modified. The proxy response pipeline chains withFooterRevision before the existing withRouterRevision to apply both the footer link and the x-uos-router-revision header. Non-HTML responses pass through unchanged except for the header. Test coverage validates HTML rewriting, existing link updates, footer appending, and non-HTML pass-through.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: append router revision link to HTML footers' directly describes the main change: injecting revision links into HTML footers as shown in the changeset.
Description check ✅ Passed The description clearly outlines the key changes: injecting revision links, preserving non-HTML behavior, and removing stale Content-Length headers, all reflected in the code changes.
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 unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: af87dd72-fb5e-45ed-b3d7-73e85ab4f8b4

📥 Commits

Reviewing files that changed from the base of the PR and between 91bebfb and bc51f59.

📒 Files selected for processing (2)
  • src/worker.ts
  • tests/worker-routing.test.ts

Comment thread src/worker.ts
Comment on lines +156 to +164
async function withFooterRevision(response: Response): Promise<Response> {
const contentType = response.headers.get('content-type') || ''
if (!/\btext\/html\b/i.test(contentType)) return response

const html = await response.text()
const rewritten = injectFooterRevision(html)
const headers = new Headers(response.headers)
if (rewritten !== html) headers.delete('content-length')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Skip footer rewrite when the upstream response has no body.

withFooterRevision rewrites any text/html response, but body-less responses (e.g., HEAD/204/304) are converted into synthetic footer HTML. That changes HTTP semantics and can drop expected headers.

Suggested fix
 async function withFooterRevision(response: Response): Promise<Response> {
   const contentType = response.headers.get('content-type') || ''
   if (!/\btext\/html\b/i.test(contentType)) return response
+  if (response.body === null || response.status === 204 || response.status === 304) {
+    return response
+  }

   const html = await response.text()
   const rewritten = injectFooterRevision(html)
   const headers = new Headers(response.headers)
   if (rewritten !== html) headers.delete('content-length')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function withFooterRevision(response: Response): Promise<Response> {
const contentType = response.headers.get('content-type') || ''
if (!/\btext\/html\b/i.test(contentType)) return response
const html = await response.text()
const rewritten = injectFooterRevision(html)
const headers = new Headers(response.headers)
if (rewritten !== html) headers.delete('content-length')
async function withFooterRevision(response: Response): Promise<Response> {
const contentType = response.headers.get('content-type') || ''
if (!/\btext\/html\b/i.test(contentType)) return response
if (response.body === null || response.status === 204 || response.status === 304) {
return response
}
const html = await response.text()
const rewritten = injectFooterRevision(html)
const headers = new Headers(response.headers)
if (rewritten !== html) headers.delete('content-length')

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.

1 participant