humanmade / humanmade/S3-Uploads

get_s3_location_for_url() doesn't recognise regional S3 hostnames

Open Beginner friendly
#746 1 comment 0 reactions 0 assignees View on GitHub
Bug Enhancement
Dominant language
PHP
Stars
2.2k
Forks
404
PR merge metrics
No merged PRs in 30d

Description

## `get_s3_location_for_url()` doesn't recognise regional S3 hostnames (`bucket.s3.{region}.amazonaws.com`)

`Plugin::get_s3_location_for_url()` (`inc/class-plugin.php` around line 280) only resolves URLs that match one of two patterns:

1. The legacy global S3 form: `https://{bucket}.s3.amazonaws.com/...`
2. The site's `wp_upload_dir()['baseurl']`.

It does **not** recognise the standard **regional** S3 form: `https://{bucket}.s3.{region}.amazonaws.com/...`. As a result, any URL that arrives in this form passes through `add_s3_signed_params_to_attachment_url()` unchanged (because `get_s3_location_for_url()` returns `null`), and the file ends up referenced by an unsigned URL — broken for any private attachment.

### How we hit it

In an environment configured with `S3_UPLOADS_REGION=eu-west-1` (or similar), `wp_get_attachment_url($id)` for a private PDF returns a presigned URL like:

```
https://hmn-uploads-eu.s3.eu-west-1.amazonaws.com/uploads/2026/04/my-test-ebook.pdf?X-Amz-Signature=...
```

WordPress core's `image_downsize()` then derives the cover-image sub-size URL by taking `dirname()` of that URL and substituting the cover JPEG filename, dropping the query string in the process:

```
https://hmn-uploads-eu.s3.eu-west-1.amazonaws.com/uploads/2026/04/my-test-ebook-pdf-106x150.jpg
```

This unsigned URL is then passed through the `wp_get_attachment_image_src` filter chain. S3 Uploads' `add_s3_signed_params_to_attachment_image_src()` calls `add_s3_signed_params_to_attachment_url()` → `get_s3_location_for_url()` → returns `null` because neither prefix check matches → URL is returned unsigned → browser hits 403 → media library tile renders broken.

We worked around this in our private-media module by rewriting the URL host to the upload baseurl before calling `add_s3_signed_params_to_attachment_url()`, but that workaround shouldn't be necessary — `get_s3_location_for_url()` should resolve regional S3 URLs natively, since the regional form is the AWS-recommended default and what the AWS SDK signs against.

### Suggested fix

Make the first check in `get_s3_location_for_url()` match both forms — something like:

```php
// Match both legacy and regional S3 hostnames:
// https://{bucket}.s3.amazonaws.com/...
// https://{bucket}.s3.{region}.amazonaws.com/...
$bucket = $this->get_s3_bucket();
$pattern = '#^https?://' . preg_quote( $bucket, '#' ) . '\.s3(\.[a-z0-9-]+)?\.amazonaws\.com/#i';
if ( preg_match( $pattern, $url ) ) {
$parsed = wp_parse_url( $url );
return [
'bucket' => $bucket,
'key' => isset( $parsed['path'] ) ? ltrim( $parsed['path'], '/' ) : '',
'query' => $parsed['query'] ?? null,
];
}
```

Optionally also accept the path-style form `https://s3.{region}.amazonaws.com/{bucket}/...` and any custom endpoint configured via `S3_UPLOADS_ENDPOINT` (relevant for non-AWS S3-compatible services like MinIO and VersityGW).

### Reproduction

1. Configure S3 Uploads with a non-`us-east-1` region.
2. Mark an attachment private (e.g. via `s3_uploads_is_attachment_private` filter).
3. Call `$plugin->get_s3_location_for_url('https://your-bucket.s3.eu-west-1.amazonaws.com/uploads/some.jpg')`.
4. Observe `null` is returned.

### Impact

Anything that derives a sub-size or sibling URL from `wp_get_attachment_url()` and then expects S3 Uploads to sign it will silently produce broken URLs in regional environments. PDFs (cover images), videos (poster images), and any custom code paths constructing URLs by manipulating the parent's URL are affected.

---

May be the same as https://github.com/humanmade/S3-Uploads/issues/406

### Acceptance Crtiteria
- [ ] Check if the steps to reproduce now provide proper result
- [ ] Check if everything else works properly as well
- [ ] Check if https://github.com/humanmade/S3-Uploads/issues/406 is covered by this fix

### Ready for Work Checklist

Is this ticket ready to be worked on? See
[the Play Book Definition of Ready](https://playbook.hmn.md/play/product/definition-of-ready/)

- [x] Is the title clear?
- [x] Is the description clear and detailed enough?
- [x] Are acceptance criteria listed?
- [x] Have any dependencies been identified? (Optional)
- [x] Have any documentation/playbook changes been identified? (Optional)
- [x] Is an estimate or time box assigned?
- [x] Is a priority label assigned?
- [ ] Is this ticket added to a milestone?
- [ ] Is this ticket added to an epic? (Optional)

### Completion Checklist

Is this ticket done? See
[the Play Book Definition of Done](https://playbook.hmn.md/play/product/definition-of-done-2/)

- [ ] Has the acceptance criteria been met?
- [ ] Is the documentation updated (including README)?
- [ ] Do any code/documentation changes meet project standards?
- [ ] Are automatic tests in place to verify the fix or new functionality?
- [ ] Or are manual tests documented (at least on this ticket)?
- [ ] Are any Playbook/Handbook pages updated?
- [ ] Has a new module release (patch/minor) been created/scheduled?
- [ ] Have the appropriate `backport` labels been added to the PR?
- [ ] Is there a roll-out (and roll-back) plan if required?

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in inc/class-plugin.php around get_s3_location_for_url() and trace how add_s3_signed_params_to_attachment_url() uses its result. Reproduce the regional hostname case from the issue, then verify regional URLs resolve and are signed correctly without regressing legacy URLs; also check whether issue 406 is covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, php
Domain
cloud
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.