humanmade / S3-Uploads

The WordPress Plugin to Store Uploads on Amazon S3
1.93k stars 388 forks source link

Sideloading images when using S3_UPLOADS_USE_LOCAL fails #223

Open tomtoday opened 6 years ago

tomtoday commented 6 years ago

I am using S3-Uploads. It works great for us, much better that other plugins we've used recently. So, to begin, thank you!

I have run into an issue trying to us the S3_UPLOADS_USE_LOCAL for offline development. We use using a plugin to import content from GatherContent. It takes advantage of sideloading. When I run the code that sideloads I get the following error:

( ! ) Warning: copy(s3://local-s3-bucket/uploads/tmp/1515881838_3861_ca32bc8c40f2ed294b779cfe5bd0ce52-DkRpxx.tmp): failed to open stream: S3_Uploads_Local_Stream_Wrapper::stream_open call failed in /application/www/wp-content/plugins/S3-Uploads/inc/class-s3-uploads.php on line 216
Call Stack
#   Time    Memory  Function    Location
1   0.0006  360752  {main}( )   .../admin-post.php:0
2   0.6452  7365760 do_action( )    .../admin-post.php:50
3   0.6452  7366136 WP_Hook->do_action( )   .../plugin.php:453
4   0.6452  7366136 WP_Hook->apply_filters( )   .../class-wp-hook.php:310
5   0.6453  7367264 GatherContent\Importer\Sync\Async_Pull_Action->handle_postback( )   .../class-wp-hook.php:286
6   0.6453  7367640 GatherContent\Importer\Sync\Async_Pull_Action->run_action( )    .../async-base.php:124
7   0.6453  7367688 GatherContent\Importer\Sync\Async_Pull_Action->run_given_action( )  .../async-base.php:160
8   0.6460  7374336 do_action( )    .../async-base.php:174
9   0.6460  7374712 WP_Hook->do_action( )   .../plugin.php:453
10  0.6460  7374712 WP_Hook->apply_filters( )   .../class-wp-hook.php:310
11  0.6460  7375840 GatherContent\Importer\Sync\Pull->sync_items( ) .../class-wp-hook.php:286
12  0.6460  7375840 GatherContent\Importer\Sync\Pull->_sync_items( )    .../base.php:144
13  0.6508  7383104 GatherContent\Importer\Sync\Pull->do_item( )    .../base.php:185
14  1.7875  10338784    GatherContent\Importer\Sync\Pull->sideload_attachments( )   .../pull.php:181
15  1.7875  10338784    GatherContent\Importer\Sync\Pull->maybe_sideload_file( )    .../pull.php:635
16  1.7902  10357560    GatherContent\Importer\Sync\Pull->sideload_file( )  .../pull.php:779
17  2.6856  10359912    media_handle_sideload( )    .../pull.php:820
18  2.6858  10360384    wp_handle_sideload( )   .../media.php:411
19  2.6858  10360384    _wp_handle_upload( )    .../file.php:951
20  2.6858  10360440    apply_filters( )    .../file.php:719
21  2.6858  10360840    WP_Hook->apply_filters( )   .../plugin.php:203
22  2.6858  10362344    S3_Uploads->filter_sideload_move_temp_file_to_s3( ) .../class-wp-hook.php:286
23  2.6860  10366160    copy ( )    .../class-s3-uploads.php:216

My configuration is as follows:

define( 'S3_UPLOADS_BUCKET', 'local-s3-bucket' );
define( 'S3_UPLOADS_KEY', 'ignoreme' );
define( 'S3_UPLOADS_SECRET', 'ignoreme' );
define( 'S3_UPLOADS_REGION', 'us-east-1' );
define('S3_UPLOADS_USE_LOCAL', true);

Normal uploads work as expected under this configuration.

tomtoday commented 6 years ago

I think I have a fix for this issue.

The code in question is a action hooked to wp_handle_sideload_prefilter called filter_sideload_move_temp_file_to_s3 here: https://github.com/humanmade/S3-Uploads/blob/master/inc/class-s3-uploads.php#L204-L221

If you remove the line where this action is added here: https://github.com/humanmade/S3-Uploads/blob/master/inc/class-s3-uploads.php#L54

Everything works as expected when using S3_UPLOADS_USE_LOCAL when the action on line 54 is removed

JoelBernerman commented 6 years ago

Does that work in production? It will probably mess up the stuff the filter is supposed to fix.

Another solution would be to check if S3_UPLOADS_USE_LOCAL is set and true before adding the action here. https://github.com/humanmade/S3-Uploads/blob/master/inc/class-s3-uploads.php#L54

That way it will use the filter when not using S3_UPLOADS_USE_LOCAL.

Just commenting as i have a issue with this filter aswell :)

tomtoday commented 6 years ago

@JoelBernerman removing the filter works in production for me. "In production" meaning "actually send files to S3 rather than store them locally using the stream handler that S3_UPLOADS_USE_LOCAL invokes.

The reason that the filter was put in place to workaround a Wordpress core issue. That issue has since been fixed in the core. I am running WP 4.9.x

JoelBernerman commented 6 years ago

@tomtoday Hmm that seemed to fix my issue too. Using sideload in ajax calls.