Skip to content

fix: resolve empty attachment MIME types from filename#9424

Open
pranav-afk wants to merge 1 commit into
makeplane:previewfrom
pranav-afk:fix/9026-empty-attachment-mime-type
Open

fix: resolve empty attachment MIME types from filename#9424
pranav-afk wants to merge 1 commit into
makeplane:previewfrom
pranav-afk:fix/9026-empty-attachment-mime-type

Conversation

@pranav-afk

@pranav-afk pranav-afk commented Jul 15, 2026

Copy link
Copy Markdown

Description

Fixes attachment uploads failing with "Invalid file type" when the browser sends an empty MIME type for text-like files (.md, .csv, .txt), common on macOS.

Allowlisted extensions were already supported, but the API required a non-empty type and rejected type: "". The client also often left type empty after signature detection (text files have no reliable magic bytes).

Changes:

  • Resolve content type from filename when the client omits it (mimetypes + extension fallbacks)
  • Wire the helper into issue attachment v2, public issue attachment, and generic asset upload endpoints
  • Allow text/csv and text/tab-separated-values in ATTACHMENT_MIME_TYPES
  • Frontend upload helper: fall back to file.type, then extension map
  • Unit tests for the MIME resolution helper

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Improvement (change that would cause existing functionality to not work as expected)
  • Code refactoring
  • Performance improvements
  • Documentation update

Test Scenarios

  • Attach .md, .csv, and .txt files to a work item (especially from macOS) — expect success, not 400 Invalid file type
  • Attach a normal image (e.g. PNG) — still works
  • Attach a disallowed extension — still rejected
  • POST attachment metadata with "type": "" and a valid name — server guesses type and accepts if allowlisted
  • Run unit tests for resolve_attachment_content_type

References

Summary by CodeRabbit

  • New Features

    • Improved attachment type detection using the file name when MIME information is missing or incomplete.
    • Added support for CSV and tab-separated values attachments.
    • Improved browser-side detection for common text-based files.
  • Bug Fixes

    • Reduced attachment upload failures caused by missing or incorrect MIME types.
    • Preserved explicitly provided content types after trimming whitespace.

Browsers on macOS often send an empty MIME type for md, csv, and txt
uploads, so the API rejected allowlisted files as Invalid file type.
Guess content type from the filename when the client omits it, add
text/csv and tsv to the allow list, and mirror the extension fallback
in the upload client helper.

Fixes makeplane#9026
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Attachment handling now resolves missing or empty MIME types from filenames across backend upload endpoints and frontend file detection, with added support for CSV and tab-separated values plus backend unit coverage.

Changes

Attachment MIME resolution

Layer / File(s) Summary
MIME resolver and supported types
apps/api/plane/utils/attachment.py, apps/api/plane/settings/common.py, apps/api/plane/tests/unit/utils/test_attachment.py
Adds filename-based MIME resolution, expands allowed text MIME types, and tests explicit-type normalization and filename fallbacks.
Backend upload integration
apps/api/plane/api/views/asset.py, apps/api/plane/api/views/issue.py, apps/api/plane/app/views/issue/attachment.py
Uses the resolver before attachment validation and asset creation across upload endpoints.
Frontend file detection
packages/services/src/file/helper.ts
Uses browser-provided MIME types or known filename extensions when signature detection does not identify a type.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • makeplane/plane#8524: Updates attachment MIME allowlisting for Markdown, which this change extends with filename-based resolution.

Suggested reviewers: dheeru0198, pablohashescobar, sriramveeraghanta

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: resolving empty attachment MIME types from filenames.
Description check ✅ Passed The description includes the required sections and covers the change, testing, type of change, and references.
Linked Issues check ✅ Passed The changes satisfy #9026 by inferring MIME types from filenames, adding client-side fallback, and preserving allowlist validation.
Out of Scope Changes check ✅ Passed No clearly unrelated changes stand out; the added helper, tests, allowlist updates, and client fallback all support the attachment MIME fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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.

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (1)
apps/api/plane/settings/common.py (1)

476-477: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicate "text/csv" entry in ATTACHMENT_MIME_TYPES.

"text/csv" is now present twice in the ATTACHMENT_MIME_TYPES list (it is already listed further down in the file). Consider removing the duplicate.

♻️ Proposed refactor
     "text/markdown",
-    "text/csv",
     "text/tab-separated-values",
     "application/rtf",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/plane/settings/common.py` around lines 476 - 477, Remove the newly
added duplicate "text/csv" entry from ATTACHMENT_MIME_TYPES, keeping the
existing occurrence elsewhere in the list and retaining
"text/tab-separated-values".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/api/plane/settings/common.py`:
- Around line 476-477: Remove the newly added duplicate "text/csv" entry from
ATTACHMENT_MIME_TYPES, keeping the existing occurrence elsewhere in the list and
retaining "text/tab-separated-values".

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a83c9704-a1a1-4a18-ab49-34c24a93bb3a

📥 Commits

Reviewing files that changed from the base of the PR and between bed58d9 and 90c877e.

📒 Files selected for processing (7)
  • apps/api/plane/api/views/asset.py
  • apps/api/plane/api/views/issue.py
  • apps/api/plane/app/views/issue/attachment.py
  • apps/api/plane/settings/common.py
  • apps/api/plane/tests/unit/utils/test_attachment.py
  • apps/api/plane/utils/attachment.py
  • packages/services/src/file/helper.ts

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.

[bug]: Attachments fail with "Invalid file type" when browser sends empty MIME (.md/.csv/.txt on macOS)

1 participant