fix(sync): panic on s3 URI with query string#1974
Conversation
✅ Deploy Preview for polite-licorice-3db33c canceled.
|
dosubot
Bot
added
the
size:M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new internal package 'bloburi' to centralize and simplify the parsing and reconstruction of blob sync URIs (such as S3, GCS, and Azure Blob). It refactors 'blob_sync.go' and 'syncbuilder.go' to use the new 'Split' and 'Join' helpers, ensuring query parameters are correctly preserved. Unit tests are also added to verify this behavior. The review feedback suggests adding a nil check for 'schemeRegex' and validating that 'bucket' is not empty in 'Split' to prevent potential nil pointer dereferences and handle invalid URIs gracefully.
toddbaert
force-pushed
the
fix/s3-sync-source-uri-panic
branch
from
d9cf105 to
a3c3657
Compare
Verified locally against MinIO. Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
force-pushed
the
fix/s3-sync-source-uri-panic
branch
from
a3c3657 to
0d6a5ee
Compare
| @@ -0,0 +1,32 @@ | |||
| // Package bloburi splits and rejoins blob sync URIs (s3://, gs://, azblob://). | |||
There was a problem hiding this comment.
I extracted these because they need to be inverses of each-other for the logic to work. I've added tests that ensure that as well.
So flagd picks up the core fix from #1973 in the next release. Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
|
🤖 I have created a release *beep* *boop* --- <details><summary>flagd: 0.15.7</summary> ## [0.15.7](flagd/v0.15.6...flagd/v0.15.7) (2026-05-29) ### 🐛 Bug Fixes * **sync:** panic on s3 URI with query string ([#1974](#1974)) ([82040ff](82040ff)) </details> <details><summary>core: 0.15.8</summary> ## [0.15.8](core/v0.15.7...core/v0.15.8) (2026-05-29) ### 🐛 Bug Fixes * **sync:** panic on s3 URI with query string ([#1974](#1974)) ([82040ff](82040ff)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
3 participants
#1971 put the query string on the bucket URL (so gocloud reads it), but
blob.Syncstill emitsSourceasBucket + Object, so it no longer matches the registered URI andStore.Updatepanics.Pulled the split/join into
internal/bloburiand rebuild the URI when emittingDataSync.Sourceso that things stay consistent if there's changes here - added a test for that as well to lock them as inverses.Verified manually/locally against MinIO.
Closes #1973
FYI @mvanhorn