Skip to content

fix(avatar): correct 0-byte Content-Length header omission#39188

Open
Harshit2405-2004 wants to merge 1 commit intoRocketChat:developfrom
Harshit2405-2004:fix/avatar-zero-size-content-length
Open

fix(avatar): correct 0-byte Content-Length header omission#39188
Harshit2405-2004 wants to merge 1 commit intoRocketChat:developfrom
Harshit2405-2004:fix/avatar-zero-size-content-length

Conversation

@Harshit2405-2004
Copy link

@Harshit2405-2004 Harshit2405-2004 commented Feb 28, 2026

Closes #39187

Summary by CodeRabbit

  • Bug Fixes
    • Fixed missing Content-Length HTTP header when serving zero-size avatar files.

@Harshit2405-2004 Harshit2405-2004 requested a review from a team as a code owner February 28, 2026 10:32
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Feb 28, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Feb 28, 2026

🦋 Changeset detected

Latest commit: ba8baa3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 41 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/ui-voip Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/abac Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/license Patch
@rocket.chat/media-calls Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch
@rocket.chat/server-fetch Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 28, 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 3c30636 and ba8baa3.

📒 Files selected for processing (3)
  • .changeset/fix-avatar-zero-size-header.md
  • apps/meteor/server/routes/avatar/utils.spec.ts
  • apps/meteor/server/routes/avatar/utils.ts
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/server/routes/avatar/utils.ts
  • apps/meteor/server/routes/avatar/utils.spec.ts
**/*.spec.ts

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use .spec.ts extension for test files (e.g., login.spec.ts)

Files:

  • apps/meteor/server/routes/avatar/utils.spec.ts
🧠 Learnings (5)
📚 Learning: 2026-01-26T18:26:01.279Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38227
File: apps/meteor/app/api/server/router.ts:44-49
Timestamp: 2026-01-26T18:26:01.279Z
Learning: In apps/meteor/app/api/server/router.ts, when retrieving bodyParams and queryParams from the Hono context via c.get(), do not add defensive defaults (e.g., ?? {}). The code should fail fast if these parameters are missing, as endpoint handlers expect them to be present and breaking here helps surface parsing problems rather than hiding them.

Applied to files:

  • apps/meteor/server/routes/avatar/utils.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

  • apps/meteor/server/routes/avatar/utils.ts
  • apps/meteor/server/routes/avatar/utils.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

  • apps/meteor/server/routes/avatar/utils.ts
  • apps/meteor/server/routes/avatar/utils.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files

Applied to files:

  • apps/meteor/server/routes/avatar/utils.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.

Applied to files:

  • apps/meteor/server/routes/avatar/utils.spec.ts
🧬 Code graph analysis (1)
apps/meteor/server/routes/avatar/utils.spec.ts (1)
apps/meteor/server/routes/avatar/utils.ts (1)
  • serveAvatarFile (24-48)
🔇 Additional comments (3)
apps/meteor/server/routes/avatar/utils.ts (1)

43-45: LGTM!

The fix correctly addresses the issue by using typeof file.size === 'number' instead of a truthy check. This ensures Content-Length: 0 is properly set for zero-byte files while still guarding against undefined or non-numeric values.

apps/meteor/server/routes/avatar/utils.spec.ts (1)

84-96: LGTM!

The test case properly verifies the fix by:

  1. Creating a zero-size file object
  2. Calling serveAvatarFile with it
  3. Confirming the Content-Length header is set to 0

The use of getCalls().find() to locate the specific header call is a robust approach that doesn't rely on call order.

.changeset/fix-avatar-zero-size-header.md (1)

1-5: LGTM!

The changeset correctly documents this as a patch-level fix with a clear, concise description of the bug being addressed.


Walkthrough

This change fixes a bug where the Content-Length HTTP header was not being set when serving avatar files with zero bytes. The condition is updated to explicitly check if file size is a number instead of relying on JavaScript truthy evaluation, which treats 0 as falsy.

Changes

Cohort / File(s) Summary
Documentation
.changeset/fix-avatar-zero-size-header.md
Adds a changeset entry documenting the patch that fixes the Content-Length header omission for zero-byte avatar files.
Testing
apps/meteor/server/routes/avatar/utils.spec.ts
Adds new test case to verify that serveAvatarFile correctly sets Content-Length header to 0 when avatar file size is exactly 0 bytes.
Implementation
apps/meteor/server/routes/avatar/utils.ts
Changes the condition from if (file.size) to if (typeof file.size === 'number') to properly handle zero-byte files and ensure Content-Length header is set for all valid numeric sizes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: fixing the omission of Content-Length header when avatar file size is zero, which aligns with the changeset and code modifications.
Linked Issues check ✅ Passed The code changes correctly implement the proposed fix from issue #39187 by replacing the truthy check with an explicit type check for numeric size values.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the avatar Content-Length header issue: the fix in utils.ts, corresponding test cases, and the changeset documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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


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.

@Harshit2405-2004 Harshit2405-2004 force-pushed the fix/avatar-zero-size-content-length branch from 4e11b97 to ba8baa3 Compare February 28, 2026 10:32
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 5 files

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.

Backend [avatar]: Content-Length header is violently stripped when avatar file size is zero

1 participant