<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 2025/9/30 17:06, Gao Xiang wrote:<br>
</div>
<blockquote type="cite"
cite="mid:8c2c314c-3088-4e25-ab20-63ac6402b004@linux.alibaba.com">Hi
Yifan,
<br>
<br>
On 2025/9/30 16:40, Yifan Zhao wrote:
<br>
<blockquote type="cite">From: zhaoyifan
<a class="moz-txt-link-rfc2396E" href="mailto:zhaoyifan28@huawei.com"><zhaoyifan28@huawei.com></a>
<br>
<br>
`mkfs.erofs` failed to generate image from Huawei OBS with the
following command:
<br>
<br>
mkfs.erofs --s3=<endpoint>,urlstyle=vhost,sig=2
s3.erofs test-bucket
<br>
<br>
because it mistakenly generated a url with repeated '/':
<br>
<br>
<a class="moz-txt-link-freetext" href="https://test-bucket">https://test-bucket</a>.<endpoint>//<keyname>
<br>
<br>
In fact, the splitting of bucket name and path has already been
performed prior
<br>
to the call to `s3erofs_prepare_url`, and this function does not
need to handle
<br>
this logic. This patch simplifies this part accordingly and
fixes the problem.
<br>
<br>
Fixes: 29728ba8f6f6 ("erofs-utils: mkfs: support EROFS meta-only
image generation from S3")
<br>
Signed-off-by: Yifan Zhao <a class="moz-txt-link-rfc2396E" href="mailto:zhaoyifan28@huawei.com"><zhaoyifan28@huawei.com></a>
<br>
---
<br>
lib/remotes/s3.c | 35 ++++++++++-------------------------
<br>
1 file changed, 10 insertions(+), 25 deletions(-)
<br>
<br>
diff --git a/lib/remotes/s3.c b/lib/remotes/s3.c
<br>
index 2e7763e..2bd5322 100644
<br>
--- a/lib/remotes/s3.c
<br>
+++ b/lib/remotes/s3.c
<br>
@@ -41,17 +41,16 @@ struct s3erofs_curl_request {
<br>
static int s3erofs_prepare_url(struct s3erofs_curl_request
*req,
<br>
const char *endpoint,
<br>
- const char *path, const char *key,
<br>
</blockquote>
<br>
I really think we should at least add a unittest for this. <br>
<br>
</blockquote>
<p>Hi Xiang,</p>
<p><br>
</p>
<p
style="--tw-border-spacing-x: 0; --tw-border-spacing-y: 0; --tw-translate-x: 0; --tw-translate-y: 0; --tw-rotate: 0; --tw-skew-x: 0; --tw-skew-y: 0; --tw-scale-x: 1; --tw-scale-y: 1; --tw-pan-x: ; --tw-pan-y: ; --tw-pinch-zoom: ; --tw-scroll-snap-strictness: proximity; --tw-gradient-from-position: ; --tw-gradient-via-position: ; --tw-gradient-to-position: ; --tw-ordinal: ; --tw-slashed-zero: ; --tw-numeric-figure: ; --tw-numeric-spacing: ; --tw-numeric-fraction: ; --tw-ring-inset: ; --tw-ring-offset-width: 0px; --tw-ring-offset-color: #fff; --tw-ring-color: rgb(59 130 246 / .5); --tw-ring-offset-shadow: 0 0 #0000; --tw-ring-shadow: 0 0 #0000; --tw-shadow: 0 0 #0000; --tw-shadow-colored: 0 0 #0000; --tw-blur: ; --tw-brightness: ; --tw-contrast: ; --tw-grayscale: ; --tw-hue-rotate: ; --tw-invert: ; --tw-saturate: ; --tw-sepia: ; --tw-drop-shadow: ; --tw-backdrop-blur: ; --tw-backdrop-brightness: ; --tw-backdrop-contrast: ; --tw-backdrop-grayscale: ; --tw-backdrop-hue-rotate: ; --tw-backdrop-invert: ; --tw-backdrop-opacity: ; --tw-backdrop-saturate: ; --tw-backdrop-sepia: ; --tw-contain-size: ; --tw-contain-layout: ; --tw-contain-paint: ; --tw-contain-style: ; box-sizing: border-box; border-width: 0px; border-style: solid; border-color: rgb(227, 227, 227); margin: 12px 0px; unicode-bidi: plaintext; font-weight: 400; font-size: 16px; line-height: 1.75; letter-spacing: 0.32px; color: rgb(44, 44, 54); font-family: system-ui, ui-sans-serif, -apple-system, BlinkMacSystemFont, sans-serif, Inter, NotoSansHans; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-line; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">I agree that adding unit tests is necessary, but the issue is that it's difficult for us to verify the validity of the URL (and accompanying request headers) unless we actually make a request to a remote S3 service. For example, the issue described in this patch only occurs with Huawei Cloud OBS, not with Alibaba Cloud OSS.</p>
<div class="my-2"
style="--tw-border-spacing-x: 0; --tw-border-spacing-y: 0; --tw-translate-x: 0; --tw-translate-y: 0; --tw-rotate: 0; --tw-skew-x: 0; --tw-skew-y: 0; --tw-scale-x: 1; --tw-scale-y: 1; --tw-pan-x: ; --tw-pan-y: ; --tw-pinch-zoom: ; --tw-scroll-snap-strictness: proximity; --tw-gradient-from-position: ; --tw-gradient-via-position: ; --tw-gradient-to-position: ; --tw-ordinal: ; --tw-slashed-zero: ; --tw-numeric-figure: ; --tw-numeric-spacing: ; --tw-numeric-fraction: ; --tw-ring-inset: ; --tw-ring-offset-width: 0px; --tw-ring-offset-color: #fff; --tw-ring-color: rgb(59 130 246 / .5); --tw-ring-offset-shadow: 0 0 #0000; --tw-ring-shadow: 0 0 #0000; --tw-shadow: 0 0 #0000; --tw-shadow-colored: 0 0 #0000; --tw-blur: ; --tw-brightness: ; --tw-contrast: ; --tw-grayscale: ; --tw-hue-rotate: ; --tw-invert: ; --tw-saturate: ; --tw-sepia: ; --tw-drop-shadow: ; --tw-backdrop-blur: ; --tw-backdrop-brightness: ; --tw-backdrop-contrast: ; --tw-backdrop-grayscale: ; --tw-backdrop-hue-rotate: ; --tw-backdrop-invert: ; --tw-backdrop-opacity: ; --tw-backdrop-saturate: ; --tw-backdrop-sepia: ; --tw-contain-size: ; --tw-contain-layout: ; --tw-contain-paint: ; --tw-contain-style: ; box-sizing: border-box; border-width: 0px; border-style: solid; border-color: rgb(227, 227, 227); margin-top: 0.5rem; margin-bottom: 0.5rem; color: rgb(44, 44, 54); font-family: system-ui, ui-sans-serif, -apple-system, BlinkMacSystemFont, sans-serif, Inter, NotoSansHans; font-size: 16px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: 0.32px; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-line; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"></div>
<p data-spm-anchor-id="a2ty_o01.29997173.0.i14.1ad3c921heqRwN"
style="--tw-border-spacing-x: 0; --tw-border-spacing-y: 0; --tw-translate-x: 0; --tw-translate-y: 0; --tw-rotate: 0; --tw-skew-x: 0; --tw-skew-y: 0; --tw-scale-x: 1; --tw-scale-y: 1; --tw-pan-x: ; --tw-pan-y: ; --tw-pinch-zoom: ; --tw-scroll-snap-strictness: proximity; --tw-gradient-from-position: ; --tw-gradient-via-position: ; --tw-gradient-to-position: ; --tw-ordinal: ; --tw-slashed-zero: ; --tw-numeric-figure: ; --tw-numeric-spacing: ; --tw-numeric-fraction: ; --tw-ring-inset: ; --tw-ring-offset-width: 0px; --tw-ring-offset-color: #fff; --tw-ring-color: rgb(59 130 246 / .5); --tw-ring-offset-shadow: 0 0 #0000; --tw-ring-shadow: 0 0 #0000; --tw-shadow: 0 0 #0000; --tw-shadow-colored: 0 0 #0000; --tw-blur: ; --tw-brightness: ; --tw-contrast: ; --tw-grayscale: ; --tw-hue-rotate: ; --tw-invert: ; --tw-saturate: ; --tw-sepia: ; --tw-drop-shadow: ; --tw-backdrop-blur: ; --tw-backdrop-brightness: ; --tw-backdrop-contrast: ; --tw-backdrop-grayscale: ; --tw-backdrop-hue-rotate: ; --tw-backdrop-invert: ; --tw-backdrop-opacity: ; --tw-backdrop-saturate: ; --tw-backdrop-sepia: ; --tw-contain-size: ; --tw-contain-layout: ; --tw-contain-paint: ; --tw-contain-style: ; box-sizing: border-box; border-width: 0px; border-style: solid; border-color: rgb(227, 227, 227); margin: 12px 0px; unicode-bidi: plaintext; font-weight: 400; font-size: 16px; line-height: 1.75; letter-spacing: 0.32px; color: rgb(44, 44, 54); font-family: system-ui, ui-sans-serif, -apple-system, BlinkMacSystemFont, sans-serif, Inter, NotoSansHans; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-line; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">I suggest that as a first step, we could perform basic rule-based validation of the URL in unit tests—such as checking whether the canonical query matches the URI, etc. And I will do it before submitting any patches modifying the url perparation logic in s3erofs implementaion.</p>
<p data-spm-anchor-id="a2ty_o01.29997173.0.i14.1ad3c921heqRwN"
style="--tw-border-spacing-x: 0; --tw-border-spacing-y: 0; --tw-translate-x: 0; --tw-translate-y: 0; --tw-rotate: 0; --tw-skew-x: 0; --tw-skew-y: 0; --tw-scale-x: 1; --tw-scale-y: 1; --tw-pan-x: ; --tw-pan-y: ; --tw-pinch-zoom: ; --tw-scroll-snap-strictness: proximity; --tw-gradient-from-position: ; --tw-gradient-via-position: ; --tw-gradient-to-position: ; --tw-ordinal: ; --tw-slashed-zero: ; --tw-numeric-figure: ; --tw-numeric-spacing: ; --tw-numeric-fraction: ; --tw-ring-inset: ; --tw-ring-offset-width: 0px; --tw-ring-offset-color: #fff; --tw-ring-color: rgb(59 130 246 / .5); --tw-ring-offset-shadow: 0 0 #0000; --tw-ring-shadow: 0 0 #0000; --tw-shadow: 0 0 #0000; --tw-shadow-colored: 0 0 #0000; --tw-blur: ; --tw-brightness: ; --tw-contrast: ; --tw-grayscale: ; --tw-hue-rotate: ; --tw-invert: ; --tw-saturate: ; --tw-sepia: ; --tw-drop-shadow: ; --tw-backdrop-blur: ; --tw-backdrop-brightness: ; --tw-backdrop-contrast: ; --tw-backdrop-grayscale: ; --tw-backdrop-hue-rotate: ; --tw-backdrop-invert: ; --tw-backdrop-opacity: ; --tw-backdrop-saturate: ; --tw-backdrop-sepia: ; --tw-contain-size: ; --tw-contain-layout: ; --tw-contain-paint: ; --tw-contain-style: ; box-sizing: border-box; border-width: 0px; border-style: solid; border-color: rgb(227, 227, 227); margin: 12px 0px; unicode-bidi: plaintext; font-weight: 400; font-size: 16px; line-height: 1.75; letter-spacing: 0.32px; color: rgb(44, 44, 54); font-family: system-ui, ui-sans-serif, -apple-system, BlinkMacSystemFont, sans-serif, Inter, NotoSansHans; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-line; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">In the future, we could integrate tools that simulate an S3 service (e.g., <a class="moz-txt-link-freetext" href="https://github.com/adobe/S3Mock">https://github.com/adobe/S3Mock</a>) as part of our CI testing pipeline (<span
style="color: rgb(44, 44, 54); font-family: system-ui, ui-sans-serif, -apple-system, BlinkMacSystemFont, sans-serif, Inter, NotoSansHans; font-size: 16px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: 0.32px; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-line; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">although this still isn't sufficient to cover the compatibility differences across various cloud providers' S3 interfaces...</span>. )</p>
<p><span
style="color: rgb(44, 44, 54); font-family: system-ui, ui-sans-serif, -apple-system, BlinkMacSystemFont, sans-serif, Inter, NotoSansHans; font-size: 16px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: 0.32px; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-line; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">What's your opinion?</span></p>
<p><br>
</p>
<p>Thanks,</p>
<p>Yifan</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:8c2c314c-3088-4e25-ab20-63ac6402b004@linux.alibaba.com">you
could simply add
<br>
<br>
#ifdef TEST
<br>
int main(int argc, char argv[])
<br>
{
<br>
testfunc1(); // and use assert() if test fails
<br>
testfunc2();
<br>
}
<br>
#endif
<br>
<br>
and use gcc -o s3_test -Iinclude -lcurl lib/remote/s3.c to
generate a test program.
<br>
<br>
Thanks,
<br>
Gao Xiang
<br>
<br>
</blockquote>
</body>
</html>