`mkfs.erofs` failed to generate image from Huawei OBS with the following 
command:

        mkfs.erofs --s3=<endpoint>,urlstyle=vhost,sig=2 s3.erofs test-bucket

because it mistakenly generated a url with repeated '/':

        https://test-bucket.<endpoint>//<keyname>

In fact, the splitting of bucket name and path has already been performed prior
to the call to `s3erofs_prepare_url`, and this function does not need to handle
this logic. This patch simplifies this part accordingly and fixes the problem.

Fixes: 29728ba8f6f6 ("erofs-utils: mkfs: support EROFS meta-only image 
generation from S3")
Signed-off-by: Yifan Zhao <[email protected]>
---
 lib/remotes/s3.c | 226 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 201 insertions(+), 25 deletions(-)

diff --git a/lib/remotes/s3.c b/lib/remotes/s3.c
index 2e7763e..0f24c65 100644
--- a/lib/remotes/s3.c
+++ b/lib/remotes/s3.c
@@ -41,17 +41,16 @@ struct s3erofs_curl_request {
 
 static int s3erofs_prepare_url(struct s3erofs_curl_request *req,
                               const char *endpoint,
-                              const char *path, const char *key,
+                              const char *bucket, const char *key,
                               struct s3erofs_query_params *params,
                               enum s3erofs_url_style url_style)
 {
        static const char https[] = "https://";;
        const char *schema, *host;
-       bool slash = false;
        char *url = req->url;
        int pos, i;
 
-       if (!endpoint || !path)
+       if (!endpoint || !bucket)
                return -EINVAL;
 
        schema = strstr(endpoint, "://");
@@ -65,30 +64,16 @@ static int s3erofs_prepare_url(struct s3erofs_curl_request 
*req,
                        return -ENOMEM;
        }
 
-       if (url_style == S3EROFS_URL_STYLE_PATH) {
-               pos = snprintf(url, S3EROFS_URL_LEN, "%s%s/%s", schema,
-                              host, path);
-       } else {
-               const char * split = strchr(path, '/');
+       if (url_style == S3EROFS_URL_STYLE_PATH)
+               pos = snprintf(url, S3EROFS_URL_LEN, "%s%s/%s/", schema, host, 
bucket);
+       else
+               pos = snprintf(url, S3EROFS_URL_LEN, "%s%s.%s/", schema, 
bucket, host);
 
-               if (!split) {
-                       pos = snprintf(url, S3EROFS_URL_LEN, "%s%s.%s/",
-                                      schema, path, host);
-                       slash = true;
-               } else {
-                       pos = snprintf(url, S3EROFS_URL_LEN, "%s%.*s.%s%s",
-                                      schema, (int)(split - path), path,
-                                      host, split);
-               }
-       }
-       if (key) {
-               slash |= url[pos - 1] != '/';
-               pos -= !slash;
-               pos += snprintf(url + pos, S3EROFS_URL_LEN - pos, "/%s", key);
-       }
+       if (key)
+               pos += snprintf(url + pos, S3EROFS_URL_LEN - pos, "%s", key);
 
        i = snprintf(req->canonical_query, S3EROFS_CANONICAL_QUERY_LEN,
-                    "/%s%s%s", path, slash ? "/" : "", key ? key : "");
+                    "/%s/%s", bucket, key ? key : "");
        req->canonical_query[i] = '\0';
 
        for (i = 0; i < params->num; i++)
@@ -503,7 +488,7 @@ s3erofs_create_object_iterator(struct erofs_s3 *s3, const 
char *path,
        if (prefix) {
                if (++prefix - path > S3EROFS_PATH_MAX)
                        return ERR_PTR(-EINVAL);
-               iter->bucket = strndup(path, prefix - path);
+               iter->bucket = strndup(path, prefix - 1 - path);
                iter->prefix = strdup(prefix);
        } else {
                iter->bucket = strdup(path);
@@ -763,3 +748,194 @@ err_global:
        s3erofs_curl_easy_exit(s3);
        return ret;
 }
+
+#ifdef TEST
+struct s3erofs_prepare_utl_testcase {
+       const char *name;
+       const char *endpoint;
+       const char *bucket;
+       const char *key;
+       enum s3erofs_url_style url_style;
+       const char *expected_url;
+       const char *expected_canonical;
+       int expected_ret;
+};
+
+static void run_s3erofs_prepare_url_test(const struct 
s3erofs_prepare_utl_testcase *tc)
+{
+       struct s3erofs_curl_request req = { .method = "GET" };
+       struct s3erofs_query_params params = { .num = 0 };
+       int ret;
+
+       printf("Running test: %s\n", tc->name);
+
+       ret = s3erofs_prepare_url(&req, tc->endpoint, tc->bucket, tc->key, 
&params,
+                                 tc->url_style);
+
+       if (ret != tc->expected_ret) {
+               printf("  FAILED: expected return %d, got %d\n", 
tc->expected_ret, ret);
+               return;
+       }
+
+       if (ret < 0) {
+               printf("  PASSED (expected error)\n");
+               return;
+       }
+
+       if (tc->expected_url && strcmp(req.url, tc->expected_url) != 0) {
+               printf("  FAILED: URL mismatch\n");
+               printf("    Expected: %s\n", tc->expected_url);
+               printf("    Got:      %s\n", req.url);
+               return;
+       }
+
+       if (tc->expected_canonical &&
+           strcmp(req.canonical_query, tc->expected_canonical) != 0) {
+               printf("  FAILED: Canonical query mismatch\n");
+               printf("    Expected: %s\n", tc->expected_canonical);
+               printf("    Got:      %s\n", req.canonical_query);
+               return;
+       }
+
+       printf("  PASSED\n");
+       printf("    URL: %s\n", req.url);
+       printf("    Canonical: %s\n", req.canonical_query);
+}
+
+static void test_s3erofs_prepare_url()
+{
+       struct s3erofs_prepare_utl_testcase tests[] = {
+               {
+                       .name = "Virtual-hosted style with https",
+                       .endpoint = "s3.amazonaws.com",
+                       .bucket = "my-bucket",
+                       .key = "path/to/object.txt",
+                       .url_style = S3EROFS_URL_STYLE_VIRTUAL_HOST,
+                       .expected_url =
+                               
"https://my-bucket.s3.amazonaws.com/path/to/object.txt";,
+                       .expected_canonical = "/my-bucket/path/to/object.txt",
+                       .expected_ret = 0,
+               },
+               {
+                       .name = "Path style with https",
+                       .endpoint = "s3.amazonaws.com",
+                       .bucket = "my-bucket",
+                       .key = "path/to/object.txt",
+                       .url_style = S3EROFS_URL_STYLE_PATH,
+                       .expected_url =
+                               
"https://s3.amazonaws.com/my-bucket/path/to/object.txt";,
+                       .expected_canonical = "/my-bucket/path/to/object.txt",
+                       .expected_ret = 0,
+               },
+               {
+                       .name = "Virtual-hosted with explicit https://";,
+                       .endpoint = "https://s3.us-west-2.amazonaws.com";,
+                       .bucket = "test-bucket",
+                       .key = "file.bin",
+                       .url_style = S3EROFS_URL_STYLE_VIRTUAL_HOST,
+                       .expected_url =
+                               
"https://test-bucket.s3.us-west-2.amazonaws.com/file.bin";,
+                       .expected_canonical = "/test-bucket/file.bin",
+                       .expected_ret = 0,
+               },
+               {
+                       .name = "Path style with explicit http://";,
+                       .endpoint = "http://localhost:9000";,
+                       .bucket = "local-bucket",
+                       .key = "data/file.dat",
+                       .url_style = S3EROFS_URL_STYLE_PATH,
+                       .expected_url =
+                               
"http://localhost:9000/local-bucket/data/file.dat";,
+                       .expected_canonical = "/local-bucket/data/file.dat",
+                       .expected_ret = 0,
+               },
+               {
+                       .name = "Virtual-hosted style with key ends with slash",
+                       .endpoint = "http://localhost:9000";,
+                       .bucket = "local-bucket",
+                       .key = "data/file.dat/",
+                       .url_style = S3EROFS_URL_STYLE_VIRTUAL_HOST,
+                       .expected_url =
+                               
"http://local-bucket.localhost:9000/data/file.dat/";,
+                       .expected_canonical = "/local-bucket/data/file.dat/",
+                       .expected_ret = 0,
+               },
+               {
+                       .name = "Path style with key ends with slash",
+                       .endpoint = "http://localhost:9000";,
+                       .bucket = "local-bucket",
+                       .key = "data/file.dat/",
+                       .url_style = S3EROFS_URL_STYLE_PATH,
+                       .expected_url =
+                               
"http://localhost:9000/local-bucket/data/file.dat/";,
+                       .expected_canonical = "/local-bucket/data/file.dat/",
+                       .expected_ret = 0,
+               },
+               {
+                       .name = "Virtual-hosted without key",
+                       .endpoint = "s3.amazonaws.com",
+                       .bucket = "my-bucket",
+                       .key = NULL,
+                       .url_style = S3EROFS_URL_STYLE_VIRTUAL_HOST,
+                       .expected_url = "https://my-bucket.s3.amazonaws.com/";,
+                       .expected_canonical = "/my-bucket/",
+                       .expected_ret = 0,
+               },
+               {
+                       .name = "Path style without key",
+                       .endpoint = "s3.amazonaws.com",
+                       .bucket = "my-bucket",
+                       .key = NULL,
+                       .url_style = S3EROFS_URL_STYLE_PATH,
+                       .expected_url = "https://s3.amazonaws.com/my-bucket/";,
+                       .expected_canonical = "/my-bucket/",
+                       .expected_ret = 0,
+               },
+               {
+                       .name = "Error: NULL endpoint",
+                       .endpoint = NULL,
+                       .bucket = "my-bucket",
+                       .key = "file.txt",
+                       .url_style = S3EROFS_URL_STYLE_PATH,
+                       .expected_url = NULL,
+                       .expected_canonical = NULL,
+                       .expected_ret = -EINVAL,
+               },
+               {
+                       .name = "Error: NULL bucket",
+                       .endpoint = "s3.amazonaws.com",
+                       .bucket = NULL,
+                       .key = "file.txt",
+                       .url_style = S3EROFS_URL_STYLE_PATH,
+                       .expected_url = NULL,
+                       .expected_canonical = NULL,
+                       .expected_ret = -EINVAL,
+               },
+               {
+                       .name = "Key with special characters",
+                       .endpoint = "s3.amazonaws.com",
+                       .bucket = "bucket",
+                       .key = "path/to/file-name_v2.0.txt",
+                       .url_style = S3EROFS_URL_STYLE_VIRTUAL_HOST,
+                       .expected_url =
+                               
"https://bucket.s3.amazonaws.com/path/to/file-name_v2.0.txt";,
+                       .expected_canonical = 
"/bucket/path/to/file-name_v2.0.txt",
+                       .expected_ret = 0,
+               }
+       };
+
+       int num_tests = sizeof(tests) / sizeof(tests[0]);
+
+       for (int i = 0; i < num_tests; i++) {
+               run_s3erofs_prepare_url_test(&tests[i]);
+               printf("\n");
+       }
+
+       printf("Run all %d tests\n", num_tests);
+}
+
+int main(int argc, char *argv[])
+{
+       test_s3erofs_prepare_url();
+}
+#endif
-- 
2.46.0


Reply via email to