Skip to content

Conversation

@Kleppinger
Copy link
Contributor

In #55119 it was reported that when using the S3 adapter on Windows and setting a root, the PathPrefixer uses the OS's DIRECTORY_SEPARATOR, which on Windows is \\.

However, AWS only allows / as a separator, and most S3 implementations also do.

This PR overrides the PathPrefixer for the S3 Adapter to always use /.

I have added a test to make sure this behavior works, by manually setting the directory_separator config value in the test. This ensures that / is always used.

This should not break anything.

Repost of my last pull requests because unfortunally Laravel Pint or some other IDe tool redid all the naming of the test cases.

@taylorotwell taylorotwell merged commit 570c154 into laravel:12.x Oct 22, 2025
44 of 66 checks passed
@JapSeyz
Copy link

JapSeyz commented Oct 27, 2025

I fear this is not working as intended, if I provide a prefix i the config:

laravel/framework 12.34.0
image

After running composer update laravel/framework to 12.35.1
image

Notice the duplicate path prefix.

In my filesystem, I have a prefix attribute:

 'media' => [
            'driver' => 's3',
            'key' => env('AWS_ACCESS_KEY_ID'),
            'secret' => env('AWS_SECRET_ACCESS_KEY'),
            'region' => env('AWS_DEFAULT_REGION', 'eu-central-1'),
            'bucket' => env('AWS_BUCKET'),
            'url' => env('AWS_URL'),
            'endpoint' => env('AWS_ENDPOINT'),
            'use_path_style_endpoint' => env('AWS_USE_PATH_STYLE_ENDPOINT', false),
            'prefix' => 'media',
            'throw' => true,
        ],

I've had to revert the update, since it broke every single prefixed URL in my app.

If I manually comment out this change in my vendor directory, it works as expected again.

I am using stancl/tenancy which this issue was supposed to fix something for, I believe.

@JapSeyz
Copy link

JapSeyz commented Oct 27, 2025

CC @taylorotwell It might be worth rolling this back, or releasing #57534 before too many people start updating to 12.35.1.

It took me a bit of time to figure out the bug was actually coming from laravel/framework

@sstottelaar
Copy link

After lots of digging, we found that this issue affects us as well. We’ve reverted the update as a workaround. Thanks to @JapSeyz for identifying the root cause and pointing it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants