File/media upload handled differently

[kirkbushell]

I'm not entirely sure what's going on here, but effectively what's happening is that media uploads work fine, but when files are uploaded, it seems to append the bucket to the domain when using a custom endpoint, ie:

S3_ENDPOINT=https://mydomain.nyc3.digitaloceanspaces.com

The requests for file uploads work fine (it uploads the file). It's the subsequent request posting to the server to save the files that fails:

Error executing "ListObjects" on "https://mydomain.mydomain.nyc3.digitaloceanspaces.com

I have tried all sorts of configuration changes and updates, all to no avail.

Any ideas?

[antonioribeiro]

Endpoint is not being handled/changed by Twill, so you probably need to check what Laravel or Flysystem does with it: https://flysystem.thephpleague.com/v1/docs/adapter/aws-s3-v3/

[kirkbushell]

So there's definitely something weird here - I'm using Digital Ocean's spaces, and the configuration I use on another project, does not work when using Twill. Ie.:

S3_KEY=ASDFLKJSDF
S3_SECRET=SECRET
S3_BUCKET=bucket
S3_REGION=nyc3
S3_ENDPOINT=https://nyc3.digitaloceanspaces.com

This does not work in twill, but works normally otherwise. I think it has something to do with the configuration management twill is doing to manage filesystems, and honestly I'm not entirely sure why it does that.

Now, I CAN get this to work for media, but not files, by adding my bucket to the endpoint, such as:

S3_ENDPOINT=https://bucket.nyc3.digitaloceanspaces.com

But then I'm back at the problem - media works, files do not. It's really bizarre.

The thing is, my configuration SHOULD work on the backend, but I think when setting up the frontend uploader, twill is not doing any endpoint setup as you'd expect, or it's thinking the endpoint is all that's needed, when in reality the bucket should be added.

I do not believe this is a an aws s3 adapter or flysystem issue, as I'm using both of those on other projects with exact same DO spaces configuration, and no issues.

[ifox]

Hi guys, this is all due to this change 5894ccc. See this comment: 5894ccc#r40789394 and pull request: #703. My suggestion to avoid a breaking change but officially support DO Spaces would be to add a new config property interpolate_custom_endpoint that defaults to false. This would let the s3Endpoint helper do what it currently does to support Minio locally (which was the intention of that change initially from @schaschjan) while allowing the option to be set to true for DO Spaces support. Any suggestions? I'm not fully convinced that this is the right change.

[kirkbushell]

Thanks for the clarification, @ifox - that certainly looks like it would be it.

The config idea is an interesting one, and agreed not sure it's the right idea. That said, because that helper checks for function existence first, I think for now I'll just implement my own to resolve it. Hopefully we get a long-term solution soon :)

[kirkbushell]

Yup, just implemented the function myself - thank you SO MUCH @ifox !!! That was a rough day. haha

This definitely needs to be fixed, it needs to be smarter. Basically, if the endpoint does not contain the bucket, add it - else return as-is. Happy to create a PR if needs be.

[antonioribeiro]

What do you guys think about making it full custom?

function s3Endpoint($disk = 'libraries')
{
    $config = config("filesystems.disks.{$disk}");

    // parse the custom endpoint or use the internal one
    $endpoint = parse_url(
        $config['endpoint']
        ?? Storage::disk($disk)->getAdapter()->getClient()->getEndpoint()
    );

    // an optional scheme can be set via config or extracted from the endpoint
    $scheme = $config['scheme'] ?? $endpoint['scheme'] ?? '';

    // suffixing with scheme is optional
    $scheme = $config['prefix_with_scheme'] ?? true
              ? "$scheme://"
              : '';

    // prefixing with bucket is optional
    $prefix = $config['prefix_with_bucket'] ?? true
              ? $config['bucket'] . '.'
              : '';

    // if a custom host is not set, use the one from the endpoint
    $host = $config['host'] ?? $endpoint['host'];

    // suffixing with root is optional
    $suffix = prefixWithSlash(
        ($config['suffix_with_root'] ?? false)
            ? $config['root']
            : ''
    );

    // endpoint may contain a path, suffix with it
    $path = prefixWithSlash(
        ($config['suffix_with_path'] ?? true)
            ? $config['path'] ?? $endpoint['path']
            : ''
    );

    // build the final URL
    return $scheme . $prefix . $host . $suffix . $path;
}

Max drive config:

'media_library' => [
    'driver' => 's3',
    'key' => env('AWS_ACCESS_KEY_ID'),
    'secret' => env('AWS_SECRET_ACCESS_KEY'),
    'region' => env('AWS_DEFAULT_REGION'),
    'bucket' => env('AWS_BUCKET'),
    'url' => env('AWS_URL'),
    'endpoint' => env('AWS_ENDPOINT'),
    'prefix_with_scheme' => env('AWS_PREFIX_WITH_SCHEME', true),
    'prefix_with_bucket' => env('AWS_PREFIX_WITH_BUCKET', true),
    'suffix_with_root' => env('AWS_SUFFIX_WITH_ROOT', false),
    'suffix_with_path' => env('AWS_SUFFIX_WITH_PATH', false),
    'host' => env('AWS_HOST'),
    'path' => env('AWS_PATH'),
    'scheme' => env('AWS_SCHEME', 'https'),
    'root' => env('AWS_ROOT', ''),
],

Minimal .env config:

AWS_ACCESS_KEY_ID=key_aws
AWS_SECRET_ACCESS_KEY=secret_aws
AWS_DEFAULT_REGION=region_aws
AWS_BUCKET=bucket_aws
AWS_ENDPOINT=https://endpoint.aws.com/path_aws

Results in

https://bucket_aws.endpoint.aws.com

Max .env config

AWS_ACCESS_KEY_ID=key_aws
AWS_SECRET_ACCESS_KEY=secret_aws
AWS_DEFAULT_REGION=region_aws
AWS_BUCKET=bucket_aws
AWS_ENDPOINT=https://endpoint.aws.com/path_aws
AWS_ROOT=custom-root/
AWS_HOST=custom-host.com
AWS_PATH=custom-path
AWS_SCHEME=tftp
AWS_PREFIX_WITH_SCHEME=true
AWS_PREFIX_WITH_BUCKET=true
AWS_SUFFIX_WITH_ROOT=true
AWS_SUFFIX_WITH_PATH=true

Results in

tftp://bucket_aws.custom-host.com/custom-root/custom-path

Turning scheme, bucket and root off:

custom-host.com/custom-path

[kirkbushell]

That honestly feels like overkill and really over-complicated. The issue is really just whether or not the bucket needs to be included. If an endpoint is provided, and the bucket has not been included as part of the URL, add it. Else leave as-is.

Alternatively, craft an endpoint by using something like an S3_ENDPOINT_DOMAIN env variable, and can construct an endpoint from that + the bucket?

[kirkbushell]

This is the solution I went with (updated based on recent discussions):

function s3Endpoint($disk = 'libraries')
{
    $endpoint = parse_url(config("filesystems.disks.{$disk}.endpoint"));
    $bucket = config("filesystems.disks.{$disk}.bucket");

    if (strpos($endpoint['host'], $bucket) !== 0) {
        $endpoint['host'] = "$bucket.{$endpoint['host']}";
    }

    $endpoint['scheme'] .= '://';

    return implode('', $endpoint);
}

[antonioribeiro]

I agree it's a lot, but the problem is that these S3 "API compatible" storage (Scaleway is another one) are popping up like rabbits now, and they are never 100% aligned to AWS, so we will end up having a new one, that has a slightly different URL structure, which will end up becoming a new ticket/change.

[kirkbushell]

In reality, if they're not identical to S3, they probably need to have a separate adapter for flysystem, or be handled on a case-by-case basis.

Atm we're trying to shoehorn S3 configuration in (currently) to fit different implementations, effectively trying to solve a problem in a repository that really shouldn't care.

[ifox]

I agree with @kirkbushell, if the provider is not actually S3 compatible, it should be supported by a different adapter.

I ended up reverting the s3Endpoint helper to it's previous behavior which matches your override. It always prefix the bucket name and resolves the endpoint from the Flysystem disk. Path-based bucket endpoints have been deprecated by AWS for a couple of years in preference for virtual host (subdomain) endpoints, so it is safe to assume that compatible providers would behave the same (as does Digital Ocean Spaces and most recent versions of Minio).

[willvincent]

Given that the only significant difference between the various "S3-compatibles" seems to be url structure, it doesn't seem like a terribly big ask to support the path-based buckets as an option, which should solve everyone's use case. Specify the custom endpoint, and a boolean for whether or not to use path based - as Flysystem continues to offer.

AWS may no longer use it, but Flysystem allows it and it's part of the AWS SDK, and frankly the url structure is separate from the protocol, which is the S3 compatible bit. As it stands presently, I can't use this with minio at all because subdomain endpoints is just not feasible in my development environment.

Flysystem has this solved, are you not using it under the hood? Why is this difficult? There's already a provision for setting path-based buckets with flysystem config, it's part of the default laravel filesystem config file for s3:

        's3' => [
            'driver' => 's3',
            'key' => env('AWS_ACCESS_KEY_ID'),
            'secret' => env('AWS_SECRET_ACCESS_KEY'),
            'region' => env('AWS_DEFAULT_REGION'),
            'bucket' => env('AWS_BUCKET'),
            'url' => env('AWS_URL'),
            'endpoint' => env('AWS_ENDPOINT'),
            'use_path_style_endpoint' => env('AWS_USE_PATH_STYLE_ENDPOINT', false),
        ],

Why can't we simply expose, and use that option?

Jan 16, 2020:

Starting with version 3.28.5 of the AWS SDK for PHP, S3 calls are performed with host-style URLs by default. You can change this behavior by setting use_path_style_endpoint to true in the service client options

While they may favor host-style in practice at AWS, there doesn't appear to be any indication that they're removing the use path style option.

[ifox]

Hi @willvincent, I will take more time to answer you when I have more time, but I wanted to address your point about the lack of indication that the path option might be removed. It will definitely be removed at some point, see https://aws.amazon.com/blogs/storage/update-to-amazon-s3-path-deprecation-plan/. It has been postponed, but it is still planned.

[willvincent]

Alright, so again that doesn't really say that it won't exist in the SDK. But even if that's the intent, that's not the current situation. Deprecation is delayed with no indication of how long it's delayed. The dot-named bucket support in virtual hosts isn't implemented, and there will be a full year after which before deprecation occurs.

We have also heard feedback from customers that virtual hosted-style URLs should support buckets that have dots in their names for compatibility reasons, so we’re working on developing that support. Once we do, we will provide at least one full year prior to deprecating support for path-style URLs for new buckets.

Not supporting it now, when Flysystem supports it, when it is still supported on aws - among all the rest, because it will eventually be deprecated by aws sometime, is a bit heavy handed.

and btw - your demo site is broken too. So not only can't I test things locally, I can't really even try out your demo 😁

[ifox]

I'm not sure why you feel the need to be condescending with us. We're all trying to help each other here in building something great that is entirely free and open source. You won't get anywhere doing this in the open source world. Instead, I would suggest you dig into the source and fix the problem you're identifying. As far as I know, this is not a common problem our community is facing. We'd be happy to prioritize if it was. We'd even be happy to prioritize it just for you if you were being respectful of our work.

and btw - can you share what is broken on the demo? if we don't know what's broken, we can't really fix it.

[willvincent]

I'm not trying to be condescending, apologies if it's come across that way. Just trying to understand why behavior is different here than the rest of laravel's filesystem config.

As for the demo, really a separate topic, but the "Content Editor" never fully loads. So if I click into any piece of content, then click the blue 'editor' button, I get the block editor thing, with the block types on the left, right side just has a spinner and never loads.. lots of 500 errors in the console:

[ifox]

Ha, try an incognito window. These requests are most likely failing because of a session mismatch, we'll look into it.