[PR #2169] Fix S3 key derivation #2124

New issue

Open

opened 2026-02-26 03:33:20 +03:00 by kerem · 0 comments

kerem commented

2026-02-26 03:33:20 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/koel/koel/pull/2169
Author: @RomanHargrave
Created: 12/1/2025
Status: 🔄 Open

Base: master ← Head: roman/s3

📝 Commits (2)

30ec427 Fix S3 key derivation
5f6cecb Remove S3LambdaMetadata

📊 Changes

2 files changed (+10 additions, -13 deletions)

View changed files

📝 app/Models/Song.php (+10 -6)
➖ app/Values/SongStorageMetadata/S3LambdaMetadata.php (+0 -7)

📄 Description

Description

Koel presently derives s3cs URLs incorrectly: when translating an s3://bucket/key/for/object URI to a signed URL, it effectively uses the last segment of the key as the the entire object key. For instance, given the aforementioned example URI, the generated download URL is https://bucket.{endpoint}/object when it should be https://bucket.{endpoint}/key/for/object.

This happens because of match greediness: as both capture groups contain zero-or-more wildcard (.) matches, the regex will only fill the second group as much as is necessary to satisfy a complete match, which places a / between the groups.

Motivation

Koel is in theory easier to use with preexisting object stores than Funkwhale, and this is essentially the only blocking issue.

Screenshots (if applicable)

Checklist

I've tested my changes thoroughly and added tests where applicable
I've updated relevant documentation (if any)
My code follows the project's conventions

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/koel/koel/pull/2169 **Author:** [@RomanHargrave](https://github.com/RomanHargrave) **Created:** 12/1/2025 **Status:** 🔄 Open **Base:** `master` ← **Head:** `roman/s3` --- ### 📝 Commits (2) - [`30ec427`](https://github.com/koel/koel/commit/30ec427537661dcbfa88b9eee4b10cd916ac88cf) Fix S3 key derivation - [`5f6cecb`](https://github.com/koel/koel/commit/5f6cecbba60aa73502b3ad00027bf22ccb702612) Remove S3LambdaMetadata ### 📊 Changes **2 files changed** (+10 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `app/Models/Song.php` (+10 -6) ➖ `app/Values/SongStorageMetadata/S3LambdaMetadata.php` (+0 -7) </details> ### 📄 Description  ## Description Koel presently derives s3cs URLs incorrectly: when translating an `s3://bucket/key/for/object` URI to a signed URL, it effectively uses the last segment of the key as the the entire object key. For instance, given the aforementioned example URI, the generated download URL is `https://bucket.{endpoint}/object` when it should be `https://bucket.{endpoint}/key/for/object`. This happens because of match greediness: as both capture groups contain zero-or-more wildcard (`.`) matches, the regex will only fill the second group as much as is necessary to satisfy a complete match, which places a `/` between the groups. ## Motivation Koel is in theory easier to use with preexisting object stores than Funkwhale, and this is essentially the only blocking issue. ## Screenshots (if applicable) ## Checklist - [x] I've tested my changes thoroughly and added tests where applicable - [ ] I've updated relevant documentation (if any) - [ ] My code follows the project's conventions --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

kerem added the

pull-request

label

2026-02-26 03:33:20 +03:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/koel-koel#2124

No description provided.

Rows
Columns