Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V15: Media library crashes when uploading large files #18113

Merged
merged 25 commits into from
Jan 28, 2025

Conversation

iOvergaard
Copy link
Contributor

@iOvergaard iOvergaard commented Jan 24, 2025

Description

Fixes #18012
Fixes #17493
Potentially fixes #17243

Tested in the latest Chrome:
Upload a file larger than ~50 MB and you will experience the browser tab crash.

Changes

  • Allows file sizes for Kestrel and multipart forms to be a long (64-bits) as is supported by the underlying framework
  • Changes from Fetch to XHR in the client to handle the large file uploads, as Chromium crashes with Fetch
  • Introduces generic xhrRequest and tryXhrRequest functions for developers to call API endpoints using XHR directly
  • Marks a few methods as sync (they were async before) because they were not using any awaits anywhere (could also have been a culprit of the browser crashing)

How to test

Set up Kestrel to accept files larger than 50 MB (Umbraco default). Here is how to set it to 2,5 GB:

      "Runtime": {
        "MaxRequestLength": 2097150,
        "TemporaryFileLifeTime": "0.00:30:00"
      }
  • Upload a file larger than the limit and check that an error is shown both in the placeholder as well as a notification
  • Upload a file larger than 50 MB (but smaller than the limit) and check that it completes
  • Upload many files and check that the valid files complete
  • Upload a folder with files and check that it is created as expected
  • Go to Tiptap and drop an image, check it is uploaded (using the XHR request)
  • Go to TinyMCE and drop an image, check it is uploaded (using the XHR request), also verify that the progress() callback works

@iOvergaard iOvergaard marked this pull request as ready for review January 27, 2025 16:03
Copy link
Member

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

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

Tested out, tried the test scenarios outlined, all looking good. 🚀

(Unsure what to do about the breaking API build errors.)

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.

2 participants