Additional forward slash '/' in s3 URL causing 404s

icidasset / diffuse

A music player that connects to your cloud/distributed storage.

https://diffuse.sh

Other

807 stars 67 forks source link

Additional forward slash '/' in s3 URL causing 404s #271

Closed tobbbles closed 3 years ago

tobbbles commented 3 years ago

Version: v3.0.0-beta-1

The following configuration generates a URL with an additional forward slash at the base of the url path:

results in

https://s3.eu-central-1.wasabisys.com//foobar?...

Unfortunately on Wasabi, the additional / causes a 404 not found as it is parsing the additional slash literally for the bucket name.

I'm really not familiar with elm, but I feel like the culprit of this resides here https://github.com/icidasset/diffuse/blob/b78e497a3cd389011fa96c1a201c34d48d78b2f7/src/Library/Sources/Services/AmazonS3/Presign.elm#L77-L83

icidasset commented 3 years ago

Oh whoops, thanks for creating this issue! Will create a new beta release in a bit.

icidasset commented 3 years ago

@tobbbles Made a new release https://github.com/icidasset/diffuse/releases/tag/3.0.0-beta-2 I can't seem to build anymore for Linux and Windows on this new Mac machine, so that will be up later. Also available at https://nightly.diffuse.sh/

Let me know if that works for you.

tobbbles commented 3 years ago

OK so I just tested the latest build and the base bucket url is now correct 👏

BUT, when attempting to fetch a file from the bucket there is a missing / at the end of the bucket before the file path, example:

bucket: foobar track path: Calibre/Feeling Normal/...

Attempted URL to fetch:

https://s3.eu-central-1.wasabisys.com/foobarCalibre/Feeling%20Normal/...

icidasset commented 3 years ago

OK so I just tested the latest build and the base bucket url is now correct 👏

BUT, when attempting to fetch a file from the bucket there is a missing / at the end of the bucket before the file path, example:

bucket: foobar track path: Calibre/Feeling Normal/...

Attempted URL to fetch:
https://s3.eu-central-1.wasabisys.com/foobarCalibre/Feeling%20Normal/...

Thanks! I need to write tests for this 🤦‍♂️ Ok, reuploaded everything, should be good now 😅

icidasset commented 3 years ago

@tobbbles Is this working for you now? Sorry for going over/closing this so quickly, I need to do better.

tobbbles commented 3 years ago

Hey! You did great - you do great work on this project I am very very grateful for your timely response, don't be hard on yourself :heart:

I'll try to get a local build from the master branch and revalidate the behaviour on non-aws s3 buckets.

As a side note; what would your thoughts be about creating builds from master branch commits that could be grabbed from the Actions artifacts?

icidasset commented 3 years ago

As a side note; what would your thoughts be about creating builds from master branch commits that could be grabbed from the Actions artifacts?

@tobbbles I have a Github action set up for the most part. I was trying to get an automatic https://tauri.studio/ build, but couldn't get it to work. That said, it does build Diffuse, create a zip and tar file and uploads the artifacts to a draft release. The action is at .github/workflows/native-build.yml

I'm not super familiar with Github actions, but I guess you can run that on your fork? You can strip out the tauri step, but it should work nonetheless.

I also haven't tried it in a few months, so let me know if that works for you.

:v: