humanmade / S3-Uploads

The WordPress Plugin to Store Uploads on Amazon S3
1.92k stars 389 forks source link

Media library returns error on upload. #456

Open natereist opened 3 years ago

natereist commented 3 years ago

Got this almost working, and honestly it appears to be working in one location and not another ( dev server works, productions doesn't ).

Both servers are running WP 5.5.1 on apache, and php 7.3.23, but the dev is ubuntu and production is CentOS.

Any ideas what I might be missing or how I could fix this error?

Thanks!

Brian-Menzel commented 3 years ago

Hi, two of us are also having this exact issue (labeled issue #420) and as far as i know neither of us has found a solution. I'm at my wits end and have made zero progress so please if you find a solution either post it here or over at #420.

I had this plug in working previously and don't want to have to find another solution to get S3 working. this isn't a professional project so i can take my time fortunately but I just can't figure out why it's throwing up this error...

natereist commented 3 years ago

Two things I notice in debugging this:

file_exists on line 1980 is returning false for the s3 resource, even if it does exists.

Then subsequently @mkdir_p on line 2003 also fails, somewhat predictably, since we know that folder is actually there.

I think if we can sort out why file_exists isn't reporting the truth we can solve this.

mdinescu commented 3 years ago

Is $target the S3 url? If so, I don't think wp_mkdir_p is supposed to execute on that at all. With S3 it is unnecessary to "create directories". in S3, what appears to be a directory structure is simply a key prefix.

I think the problem is earlier in the call stack, before wp_mkdir_p is invoked (maybe in wp_upload_dir(..)). At some point the the process should determine that the resource is an S3 resource and not even attempt to call wp_mkdir_p.

natereist commented 3 years ago

Yes the target is that s3 URL.

wp_upload_dir calls wp_mkdir_p if the create directory parameter is set to true here. That is where the error in the upload dir object gets set.

The interesting quirk for me is it is working on one server but not another, running the same plugins and version of WordPress.

mdinescu commented 3 years ago

yea, it is interesting -- I can reproduce this 100% of the time starting from scratch with an EC2 instance running Amazon Linux 2. I posted steps to repro here: https://github.com/humanmade/S3-Uploads/issues/420#issuecomment-664716344

It takes 10 minutes to get the stack up and running

I do agree that the fact that it works for you on one server but not the other is interesting, though in your case one server is running Ubuntu and the other CentOS and the two distros are quite different


Now, back to the issue itself, I have a suspicion that the problem was introduced by this change, about 4 months ago: https://github.com/WordPress/wordpress-develop/commit/e51a554f5da556804e05e0bb498f59473de8ce7d

natereist commented 3 years ago

Yes, the distros are different, but how would the issue not exist in both places if it was introduced in the code change?

The two servers are getting different response from file_exists() in php, meaning they are handling that differently right? I think the issue would be in a version discrepancy of php, cURL, or some other library at the server level and not in the WordPress application itself ( I could definitely be wrong though, just trying to eliminate some factors here ).

The cURL versions between my servers are different- dev 7.58 for both CLI and web (both work) production 7.19 CLI (works) and 7.72 web (doesn't work). What versions of cURL are you experiencing the issue with? after some testing figured out the curl version on the CLI and the version used by WP CLI are different, WP CLI uses the same as the web client 7.72

I put together a simple plugin with a couple tests one run with a query param in the admin and one in wp cli and the results are consistent with my other tests. If you want to test it you can get it here

I would think that function should return the same value regardless of the distro and versions of libraries unless there is a bug on that level.

natereist commented 3 years ago

It looks to me that the error I think is occurring is supposed to be solved in the stream wrapper for s3 customized for this plugin.

Regarding this comment, what do you think about that API endpoint is causing this issue?

Now, back to the issue itself, I have a suspicion that the problem was introduced by this change, about 4 months ago: WordPress/wordpress-develop@e51a554

natereist commented 3 years ago

it is not just the file_exists check, if I bypass that it fails to move the file when in the browser. I tested back to WP 5.3.1 and it the same error exists there on a clean WordPress install with no other plugins and just twenty-twenty installed.

Also switched to php 7.4 and the issue persists. Perhaps there is another dependency that is necessary for this to work we are missing?