durch / rust-s3

Rust library for interfacing with S3 API compatible services
MIT License
498 stars 195 forks source link

S3 Panics if given an empty region #303

Closed pcrumley closed 1 year ago

pcrumley commented 1 year ago

Describe the bug The lines of code inside of s3 request_trait.rs have something listed as infallible. see below

        // Since every part of this URL is either pre-encoded or statically
        // generated, there's really no way this should fail.
        let mut url = Url::parse(&url_str).expect("static URL parsing");

However, it actually is fallible if the bucket is created with a region that is an empty string, in which case the bucket.url() starts with : which will panic with an InvalidDomainCharacter error from the Url crate.

To Reproduce Create a bucket with an empty region, or for example a region set to :: and try to use. I can provide code if desired.

Expected behavior So in the awsregion crate it does state that if region is invalid then code goes boom. If panicking here is expected behavior, that's totally fine, but I think the expect language should be changed to .expect("may fail with invalid aws region string").

Personally, I'd prefer this code not to panic and instead return a result there and surface it back to user. I would be willing to provide a fix if either of these approaches would be likely to be accepted into the crate.

--

Thanks for all your hard work!

durch commented 1 year ago

@pcrumley great catch, we should not panic, I'll fix this, and see where else I've been sloppy in the past :)