Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adaptor mapping for custom S3 endpoint is incorrect #328

Open
rongcuid opened this issue Sep 18, 2022 · 6 comments · May be fixed by #399
Open

Adaptor mapping for custom S3 endpoint is incorrect #328

rongcuid opened this issue Sep 18, 2022 · 6 comments · May be fixed by #399
Labels

Comments

@rongcuid
Copy link

In setting up Upload for a self-hosted Minio S3 endpoint, I find the URL mapping is incorrect.

Bug Report

Current Behavior
The adaptor mapping generates URL to amazon AWS, instead of my custom endpoint.

Steps to Reproduce

  1. Configure adaptor to Mime ^image\/.*, S3/Compatible, Just URL
  2. Set up S3 storage settings. I used a valid service key/secret from my Minio instance, bucket flarum, and region home (which is definitely not an AWS region)
  3. Set endpoint to my Minio instance, in my case localhost:9000. This is tested to work using curl.
  4. Try to upload an image

Expected Behavior

Generate a URL like localhost:9000/flarum/path/to/image

Actual Behavior

Generated a URL to AWS

Screenshots

Screenshot from 2022-09-18 15-01-12

Environment

  • Flarum version: 1.5.0
  • Extension version: x.y.z
  • Website URL: http://home.local/flarum
  • Webserver: nginx
  • Hosting environment: self-hosted
  • PHP version: 8.1
  • Browser: Firefox
$ php flarum info
Flarum core 1.5.0
PHP version: 8.1.2
MySQL version: 5.5.5-10.6.7-MariaDB-2ubuntu1.1
Loaded extensions: Core, date, libxml, openssl, pcre, zlib, filter, hash, json, pcntl, Reflection, SPL, session, standard, sodium, mysqlnd, PDO, xml, calendar, ctype, curl, dom, mbstring, FFI, fileinfo, ftp, gd, gettext, iconv, intl, exif, mysqli, pdo_mysql, pdo_pgsql, pgsql, Phar, posix, readline, shmop, SimpleXML, soap, sockets, sysvmsg, sysvsem, sysvshm, tokenizer, xmlreader, xmlrpc, xmlwriter, xsl, zip, Zend OPcache
+----------------------+---------+--------+
| Flarum Extensions    |         |        |
+----------------------+---------+--------+
| ID                   | Version | Commit |
+----------------------+---------+--------+
| flarum-flags         | v1.5.0  |        |
| fof-upload           | 1.2.3   |        |
| flarum-tags          | v1.5.0  |        |
| flarum-suspend       | v1.5.0  |        |
| flarum-subscriptions | v1.5.0  |        |
| flarum-sticky        | v1.5.0  |        |
| flarum-statistics    | v1.5.0  |        |
| flarum-mentions      | v1.5.0  |        |
| flarum-markdown      | v1.5.0  |        |
| flarum-lock          | v1.5.0  |        |
| flarum-likes         | v1.5.0  |        |
| flarum-lang-english  | v1.5.0  |        |
| flarum-emoji         | v1.5.0  |        |
| flarum-bbcode        | v1.5.0  |        |
| flarum-approval      | v1.5.0  |        |
+----------------------+---------+--------+
Base URL: https://home.local/flarum
Installation path: /var/www/flarum
Queue driver: sync
Mail driver: mail
Debug mode: ON

Don't forget to turn off debug mode! It should never be turned on in a production system.

Possible solution(s)

Use custom endpoint when generating the URL

Additional Context
Add any other context about the problem here.

@rongcuid rongcuid added the bug label Sep 18, 2022
@rongcuid
Copy link
Author

Relavent code under src/Adapters/AwsS3.php:

    protected function generateUrl(File $file)
    {
        /** @var SettingsRepositoryInterface $settings */
        $settings = resolve(SettingsRepositoryInterface::class);

        $cdnUrl = $settings->get('fof-upload.cdnUrl');

        if (!$cdnUrl) {
            $region = $this->adapter->getClient()->getRegion();
            $bucket = $this->adapter->getBucket();

            $cdnUrl = sprintf('https://%s.s3.%s.amazonaws.com', $bucket, $region ?: 'us-east-1');
        }

        $file->url = sprintf('%s/%s', $cdnUrl, Arr::get($this->meta, 'path', $file->path));
    }

It might be a bad idea to use fof-upload.cdnUrl, because I think this is the setting for Local adapter. In addition, path-style endpoint does not have an effect here.

Now, my Minio is behind a reverse proxy. So the backend accesses through localhost:9000, but the frontend accesses through home.local/minio. I would suggest adding a setting that allows something like $frontend_endpoint/$bucket/$path.

I also had problem when I omit regions (which might not be set up in Minio), but this may be a separate issue.

@clarkwinkelmann
Copy link
Member

It might be a bad idea to use fof-upload.cdnUrl

Are you using both local and s3 at the same time?


Regarding the wrong URL issues I'm sure we will need to overhaul the S3 adapter at some point, it's not the first issue reported to us and I'm not sure yet how we can fix it to accommodate all "compatible" S3 services.

The workaround in the meantime would be to create a custom adapter, you could copy the S3 adapter code in it and hard-code the values.

@luceos
Copy link
Contributor

luceos commented Sep 19, 2022

The driver is accurately named AwsS3 not S3Compatible or similar. So I think the adapter does what it needs to do.

What is reported:

  • using a s3 compatible storage behind a proxy
  • using the AwsS3 adapter for a s3 compatible storage

I think that we do need a s3 compatible adapter at some point. We also talked about allowing a CDN Url per adapter. As both of these aren't available right now the best solution would be to create your own Adapter in your Flarum install or as an extension by listening to the Collecting event or by extending the Adapter Manager. In this case we should also improve extensibility a bit in a future revision.

@rongcuid
Copy link
Author

It might be a bad idea to use fof-upload.cdnUrl

Are you using both local and s3 at the same time?

Regarding the wrong URL issues I'm sure we will need to overhaul the S3 adapter at some point, it's not the first issue reported to us and I'm not sure yet how we can fix it to accommodate all "compatible" S3 services.

The workaround in the meantime would be to create a custom adapter, you could copy the S3 adapter code in it and hard-code the values.

No, but it is under local's setting, so I assumed it affects local adapter only until I saw the code. I am also unsure whether local and S3 can be used simultaneously, because I see that I can set adapter based on MIME type. Certainly there is a use case where different adapters use different CDN url, right...?

I am new to S3, so I have to figure out a lot of things myself, so I cannot say for sure what to change, except maybe set URL to $endpoint/bucket/path if $endpoint is set.

I am also new to Flarum and its ecosystem, so I have not looked into how to extending an extension yet.

@jeff-h
Copy link

jeff-h commented Apr 27, 2023

I'm a little confused; the config UI contains fields described as such "The following settings are only required when using S3 compatible storage. If you use AWS, you can leave them blank."

This obviously implies S3-compatible storage does work, but the comments on this issue suggest otherwise. In my experience, it does indeed work, with the single exception of the bug relating to the URL generation.

@jeff-h
Copy link

jeff-h commented Mar 20, 2024

I've had another crack at getting my install to work, and thanks to the code snippet posted above by @rongcuid I tried putting my Backblaze "S3 Url" into FoF Upload's "Local storage settings: Content Delivery URL (prefixes files)" config, and downloading works now! (I don't use local storage, as I'm with FreeFlarum.)

In Backblaze:

  • log into your account
  • Click "Browse files" and navigate to the uploaded file, and click on it
  • You should get a modal dialogue titled "Details"
  • look for the "S3 Url". It will look something like this: https://my-forum-downloads-bucket.s3.us-west-002.backblazeb2.com/2024-03-20/7325382501-237129-test.zip
  • in this example, copy https://my-forum-downloads-bucket.s3.us-west-002.backblazeb2.com

In the FoF Upload config:

  • Find "Local storage settings"
  • Paste the above domain into "Content Delivery URL (prefixes files)"

@iamdarkle iamdarkle linked a pull request Apr 27, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants