Skip to content

Conversation

@4levels
Copy link

@4levels 4levels commented May 10, 2021

When files are passed in as Buffers, there's no path attribute and this throws.

What does it do?

Allow Buffer argument to the enhanceFile method by testing if the passed file object has a path attribute.

Why is it needed?

Without this, the Upload service fails at the enhanceFile function, making it impossible to handle Buffers. Adding this allows both Buffers & Files to be accepted and handled properly.

How to test it?

Use the service and pass it a Buffer instead of a File, eg a request promise with remote image from a URL

Related issue(s)/PR(s)

No related issue or PR at first glance, please add if existing

When files are passed in as Buffers, there's no path attribute and this throws.  Would benefit from proper type checking..
@strapi-cla
Copy link

strapi-cla commented May 10, 2021

CLA assistant check
All committers have signed the CLA.

@derrickmehaffy
Copy link
Member

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/strapi-upload-service-cannot-handle-buffers/4830/1

Copy link
Author

@4levels 4levels left a comment

Choose a reason for hiding this comment

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

I clicked the Merge button by accident, didn't want to merge myself but it showed on Github as a required action to complete before the test builds would run.. my bad

@codecov
Copy link

codecov bot commented May 10, 2021

Codecov Report

Merging #10283 (cfc7302) into master (43b947b) will not change coverage.
The diff coverage is 0.00%.

❗ Current head cfc7302 differs from pull request most recent head 395f30d. Consider uploading reports for the commit 395f30d to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master   #10283   +/-   ##
=======================================
  Coverage   57.82%   57.82%           
=======================================
  Files         184      184           
  Lines        6393     6393           
  Branches     1395     1396    +1     
=======================================
  Hits         3697     3697           
+ Misses       2233     2232    -1     
- Partials      463      464    +1     
Flag Coverage Δ
front ∅ <ø> (∅)
unit 57.82% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/strapi-plugin-upload/services/Upload.js 14.94% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43b947b...395f30d. Read the comment docs.

@alexandrebodin alexandrebodin added the source: core:upload Source is core/upload package label May 11, 2021
4levels added 3 commits May 11, 2021 11:26
When a buffer property exists, assign it straight to the readBuffer variable instead of passing it on to fs.readFile
@derrickmehaffy
Copy link
Member

Hi 👋
We now have released the v4. As of now, the v3 is in maintenance mode for 6 months. We will only be fixing critical and security issues on v3 from now on.
I will go ahead and close this PR. Thank you for contributing to Strapi 🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

source: core:upload Source is core/upload package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants