Skip to content

fix: skip null drag entries during file upload (fixes #326644) - #326649

Merged
vs-code-engineering[bot] merged 1 commit into
mainfrom
fix/upload-null-entry-326644-88273ae029d81eaf
Jul 20, 2026
Merged

fix: skip null drag entries during file upload (fixes #326644)#326649
vs-code-engineering[bot] merged 1 commit into
mainfrom
fix/upload-null-entry-326644-88273ae029d81eaf

Conversation

@vs-code-engineering

Copy link
Copy Markdown
Contributor

Summary

TypeError: Cannot read properties of null (reading 'name') is thrown when dropping content onto the File Explorer that is not a file-system entry (e.g. dragged text, a URL, or non-file drag data). BrowserFileUpload.doUpload builds an entries array from DataTransferItem.webkitGetAsEntry(), but that DOM API returns FileSystemEntry | null — it returns null for items that do not represent a file. The local IWebkitDataTransferItem interface incorrectly declared the return type as non-nullable, so the null was pushed into an array typed as IWebkitDataTransferItemEntry[]. Later, the parallel upload callback reads entry.name (the factory frame in the stack) and crashes on the null element.

This is a type-contract violation: the interface lied about the return type, letting a null bypass the type system at the producer (the array-building loop), and the crash surfaced far downstream at entry.name.

Fixes #326644
Recommended reviewer: @bpasero

Culprit Commit

The masking type declaration (webkitGetAsEntry(): IWebkitDataTransferItemEntry) and the entries.push(item.webkitGetAsEntry()) producer are long-standing in fileImportExport.ts (the browser upload feature authored by @bpasero). No single recent commit introduced the null path; the telemetry recent-regression/stable-anomaly signals reflect a change in drop-input mix rather than a code change to this file. Most recent touch to the file was 7e8c7bef ("debt - reduce explicit any usage", @bpasero), which did not alter this behavior.

Code Flow

flowchart TD
    A[HTMLDivElement drop handler] --> B[ExplorerDelegate.drop]
    B --> C[BrowserFileUpload.upload -> withProgress]
    C --> D[doUpload: entries.push webkitGetAsEntry]
    D -->|API returns null for non-file item| E[null pushed into entries array]
    E --> F[Limiter.queue callback / factory]
    F --> G[read entry.name on null -> TypeError]
Loading

Affected Files

  • src/vs/workbench/contrib/files/browser/fileImportExport.ts — fix the IWebkitDataTransferItem.webkitGetAsEntry return type and skip null entries when building the upload list.

Repro Steps

  1. Open a folder in VS Code (desktop or web).
  2. Drag non-file content (e.g. selected text from a web page, or a URL) onto the File Explorer and drop it.
  3. webkitGetAsEntry() returns null for that item; before the fix, doUpload pushed the null and later read entry.name, throwing TypeError: Cannot read properties of null (reading 'name').

How the Fix Works

Chosen approach (src/vs/workbench/contrib/files/browser/fileImportExport.ts):

  • Corrected the IWebkitDataTransferItem.webkitGetAsEntry return type to IWebkitDataTransferItemEntry | null, matching the real DOM contract. This removes the type-system bypass that allowed null to enter a non-nullable array.
  • In doUpload, the array-building loop now captures the result and only pushes it when it is non-null, so entries never contains a null. This fixes the bug at the data producer (where the invalid null is created) rather than guarding the crash site (entry.name), consistent with the fix-at-the-producer principle. No try/catch is added and no logService.error is touched.

After this change, the doUpload loop cannot place a null into entries, so the downstream entry.name read can no longer receive a null element.

Alternatives considered:

  • Guarding entry.name at the crash site (line 166 / the factory callback) — rejected because it patches the symptom downstream and leaves the mistyped interface producing null for every other consumer.

Recommended Owner

@bpasero — original author of the browser file upload/import-export feature and most recent committer to this file.

Generated by errors-fix · 627.1 AIC · ⌖ 33.9 AIC · ⊞ 71K ·

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 20, 2026 15:52

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

Pull request overview

Note

Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.

Updates drag-and-drop file import handling to account for webkitGetAsEntry() potentially returning null, preventing null entries from being processed downstream.

Changes:

  • Updates IWebkitDataTransferItem.webkitGetAsEntry() type to return IWebkitDataTransferItemEntry | null.
  • Filters out null entries when building the entries array, with clarifying inline comments.

@vs-code-engineering
vs-code-engineering Bot requested a review from Copilot July 20, 2026 15:53
@vs-code-engineering
vs-code-engineering Bot marked this pull request as ready for review July 20, 2026 15:53
@vs-code-engineering
vs-code-engineering Bot enabled auto-merge (squash) July 20, 2026 15:53

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@vs-code-engineering
vs-code-engineering Bot merged commit 02e7a9b into main Jul 20, 2026
29 checks passed
@vs-code-engineering
vs-code-engineering Bot deleted the fix/upload-null-entry-326644-88273ae029d81eaf branch July 20, 2026 16:59
@vs-code-engineering vs-code-engineering Bot added this to the 1.131.0 milestone Jul 20, 2026
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.

[Error] unhandlederror-Cannot read properties of null (reading 'name')

4 participants