Skip to content

Bug 2002553 - BMO rest api search: Failed to fetch key from network storage when an attachment has been deleted - #2752

Merged
dklawren merged 3 commits into
mozilla:masterfrom
dklawren:2002553
Sep 18, 2026
Merged

dklawren merged 3 commits into
mozilla:masterfrom
dklawren:2002553

Conversation

@dklawren

Copy link
Copy Markdown
Collaborator

Changed files:

  1. Bugzilla/Attachment.pm:353 — data() returns '' when datasize is 0, before touching storage. Also removes the same latent crash in Bugzilla/API/V1/Github.pm:107 and Bugzilla/Attachment/Archive.pm:52.
  2. qa/t/rest_bug_attachments.t:190-251 — "Deleted Attachment Tests" section covering GET /rest/bug/attachment/ and the reported include_fields=_all path.

suhaibmujahid and others added 3 commits September 16, 2026 10:47
Comments render markdown with cmark's safe option and every `<` escaped
beforehand, so raw HTML never reaches the parser. Instead of weakening
that, convert the four disclosure tags back to real elements after
rendering: mark the escaped tags in text nodes (skipping pre/code so the
syntax can still be documented), then re-parse so the block level
elements are lifted out of the paragraph markdown wrapped them in.

Only those four exact tags are recognized and they never carry
attributes, so no other markup can be smuggled in. The marker characters
are stripped from the input so they cannot be forged, and unbalanced tags
cannot leak an unclosed element into the page.
* upstream/master:
  Bug 1995464 - Overhaul guided bug entry form
  Bumped version to 20260916.2
  Revert "Bug 2061445 - Migrate Bugzilla (system info) REST resource to native Mojo API"
  Bumped version to 20260916.1
  Bug 2072689 - Buglist link on My Dashboard is broken
  Bug 2069466
  Bug 2060932 - Support GitHub-style collapsible sections in comments
  Bug 2072224 - Add REST auth precedence note to not-yet-migrated resources
@dklawren
dklawren requested review from Xzzz and cgsheeh and a balanced review from Copilot September 18, 2026 03:09

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

🟢 Approval recommended

No unresolved review issues were identified.

Pull request overview

Fixes REST handling for deleted attachments by avoiding storage reads when attachment size is zero.

Changes:

  • Return empty data for zero-sized attachments.
  • Add regression tests for direct attachment retrieval and _all bug responses.
File summaries
File Description
qa/t/rest_bug_attachments.t Adds deleted-attachment API regression tests.
Bugzilla/Attachment.pm Prevents storage lookups for zero-sized attachments.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Xzzz Xzzz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm r=xzz

@dklawren
dklawren merged commit aef8bc9 into mozilla:master Sep 18, 2026
8 checks passed
@dklawren
dklawren deleted the 2002553 branch September 18, 2026 15:21
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.

4 participants