laravel-notification-channels / twitter

Twitter Notifications Channel for Laravel
http://laravel-notification-channels.com
MIT License
169 stars 44 forks source link

You must supply a readable file error when using withImage and AWS S3 #64

Open mikecobb-io opened 4 years ago

mikecobb-io commented 4 years ago

The following code where $image is a publicly accessible jpeg image causes an error:

return (new TwitterStatusUpdate($message))->withImage($image);

The error is: "message": "You must supply a readable file", "exception": "InvalidArgumentException", "file": "D:\\project\\vendor\\abraham\\twitteroauth\\src\\TwitterOAuth.php"

Any thoughts as to why this is happenign?

christophrumpel commented 4 years ago

What does your $image contain? Is it working locally and is this only a problem with AWS S3?

mikecobb-io commented 4 years ago

The following works:

$image = "images/5.jpg"; return (new TwitterStatusUpdate($message))->withImage($image);

This doesn't:

$image = "https://s3.eu-central-1.amazonaws.com/xxx/images/5.jpg"; return (new TwitterStatusUpdate($message))->withImage($image);

https://s3.eu-central-1.amazonaws.com/xxx/images/5.jpg is a publicly available image and can be viewed in a browser - where xxx is the name of my S3 bucket.

christophrumpel commented 4 years ago

I can't check myself right now, but if I remember correctly you need to provide a "local" path and cannot use a URL to an image.

mikecobb-io commented 4 years ago

Ok thanks

JackWH commented 2 years ago

This is still an issue. The problem seems to be in the uploadMediaNotChunked() method, where it calls is_readable() on the image URL:

/**
 * Private method to upload media (not chunked) to upload.twitter.com.
 *
 * @param string $path
 * @param array  $parameters
 *
 * @return array|object
 */
private function uploadMediaNotChunked(string $path, array $parameters)
{
    if (
        !is_readable($parameters['media']) ||
        ($file = file_get_contents($parameters['media'])) === false
    ) {
        throw new \InvalidArgumentException(
            'You must supply a readable file',
        );
    }
    $parameters['media'] = base64_encode($file);
    return $this->http(
        'POST',
        self::UPLOAD_HOST,
        $path,
        $parameters,
        false,
    );
}

I would argue the call to is_readable() is unnecessary, as file_get_contents() will return false for unreadable remote resources anyway.

Alternatively, you could change the if (! is_readable() || ! file_get_contents()) to an &&.

christophrumpel commented 2 years ago

Thanks, how can I re-produce this issue?

JackWH commented 2 years ago

@christophrumpel If you set up a Laravel Notification class like this, then try to send it out, it should throw an error when it attempts to read the image from a remote URL:

<?php

namespace App\Notifications;

class ExampleTweet extends \Illuminate\Notifications\Notification
{
    public function via(mixed $notifiable): array
    {
        return [\NotificationChannels\Twitter\TwitterChannel::class];
    }

    public function toTwitter(mixed $notifiable): \NotificationChannels\Twitter\TwitterStatusUpdate
    {
        return (new \NotificationChannels\Twitter\TwitterStatusUpdate('Lorem ipsum dolor'))
            ->withImage('https://images.unsplash.com/photo-1595433707802-6b2626ef1c91');
    }
}

In this example I'm passing an image URL in from Unsplash. In the context of the original issue, this might be a URL to a remote image on S3. Because the file is remote (yet abstracted by Laravel's Storage class to seem local), the is_readable() call is failing.

I think changing the uploadMediaNotChunked() function as described previously would solve this, whilst remaining backwards-compatible for other use cases.

christophrumpel commented 2 years ago

Thanks, everyone, I just gave it another look. We are using the https://github.com/abraham/twitteroauth package to interact with the Twitter API. The code @JackWH was referring to is from this package.

It also seems to be a limitation from the Twitter API itself, that you can only pass local images. (https://github.com/abraham/twitteroauth/issues/824#issuecomment-601724919)

So the only thing I can think of currently would be to download the images with this package and then pass the local link. But this is actually something that should be done in the application itself because I can't tell where to download it and users also need to know that this is happening.

So the easiest option would be to download the image you need yourself and pass the local path. I know this is not a great way to handling it but I don't see a another good solution right now.

JackWH commented 2 years ago

Thanks for clarifying @christophrumpel — my mistake, I hadn't noticed the uploadMediaIfChunked() function was from outside of this package.

It looks like they don't want to change the way is_readable() works in this method. It does look like they're working on supporting alternative filesystems, though.

In the meantime, I'm working around this by simply copying the file from S3 to /tmp, then passing the temporary path in to the tweet.

christophrumpel commented 2 years ago

No worries and thanks for the update. A file class would be nice indeed.