Repository navigation
Conversation
Signed-off-by: AdamZhang <[email protected]>
🔐 Codex Security Review
|
loganj
left a comment
loganj
left a comment
There was a problem hiding this comment.
🤖 Larry and Auto: an independent adversarial review of 63218ea146cd. Source review only; we did not run it. Mobile CI has not run on this fork PR.
Approving. #8086 makes the same one-line change, and only one of the two can merge. We recommend landing this one: it was opened first, and its three tests (filename present, filename absent, and an imeta for a different URL) cover #8086's single test.
The filename is untrusted remote input, but it is safe here: _safeDownloadedFilename keeps only the last path segment, removes control characters and adds a timestamp prefix, so neither ../x nor .. can leave the temp directory. Audio links still return through _buildMedia before this branch.
| url, | ||
| auth.headersFor(url), | ||
| text, | ||
| imeta?.filename ?? text, |
There was a problem hiding this comment.
Advice: an empty filename discards the label. ImetaEntry keeps a tag part filename (key, space, no value) as ''. Then '' ?? text is '', and _safeDownloadedFilename turns it into attachment, with no extension. Suggest:
final name = imeta?.filename?.trim();
// ...
(name == null || name.isEmpty) ? text : name,Advice (follow-up is fine; this was already there before this PR): with no imeta filename, [Quarterly report](https://relay/media/<sha>.pdf) still opens as Quarterly report, with no extension. That is the #8065 symptom. Falling back to the URL's last path segment before the label, or passing imeta?.mimeType to OpenFilex.open(type:), would cover it.
Summary
When an attachment has a descriptive link label, mobile opens the downloaded file under that label and loses its original name and extension. For example,
[Quarterly report](...)opens asQuarterly reporteven when itsimetatag namesq3-report.pdf.Use the matching
imetafilename when opening the attachment, falling back to the link text when it is absent. The existing filename sanitization still applies.Related issue
Fixes #8065.
Closest related work: #7581 handles filename metadata on CLI uploads. No duplicate mobile filename fix found.
Testing
q3-report.pdf, gotQuarterly report.message_content_test.dart: 88 passed, including filename precedence, missing filename, and metadata for a different attachment.flutter analyze --no-pub, Dart format (615 files), mobile file-size check, gateway recipe checks, andgit diff --checkpassed.Full-suite failures and baseline comparison
These two failures also reproduce on the unmodified base,
33f54de:ComposeBar records, previews, uploads, and sends a voice noteComposeBar voice note rejects an existing attachment like desktopThe following seven passed in targeted reruns on both the unmodified base and this branch:
retries failed startup and closes fetch-subscribe gapack replies obey payload bounds without reopening threads600 live replies coalesce into one exact recount before reopenreconnect supersedes an in-flight overflow recount and resumes itcontinued replies do not restart an active paginated recountpermanent first channel chunk failure exhausts retries and stops exportnative retry budget is shared across all chunksThe extra file-size policy suite had 9 passes and one Windows symlink-permission failure (
surface entrypoints execute through symlinked paths).The author repeated both device tests and confirmed the results. Agent review found no blockers. There is no rendering change.
buzz-review-completed