From 1f32b0da5fd8ea567bb408babea4cc090f5587ed Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Mon, 3 Apr 2023 11:38:01 +0200 Subject: [PATCH 1/3] workaround for regression in libsmbclient 4.16.9/4.17.5 --- smbclient.c | 58 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/smbclient.c b/smbclient.c index 52b2bf1..64bb7d5 100644 --- a/smbclient.c +++ b/smbclient.c @@ -1502,6 +1502,11 @@ PHP_FUNCTION(smbclient_listxattr) RETURN_FALSE; } + +/* loop from 4K to 16M */ +#define DEFAULT_BUFFER_SIZE (4 << 10) +#define MAXIMUM_BUFFER_SIZE (32 << 20) + PHP_FUNCTION(smbclient_getxattr) { char *url, *name; @@ -1511,11 +1516,7 @@ PHP_FUNCTION(smbclient_getxattr) zval *zstate; smbc_getxattr_fn smbc_getxattr; php_smbclient_state *state; -#if PHP_MAJOR_VERSION >= 7 - zend_string *svalues = NULL; -#else char *values = NULL; -#endif if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rss", &zstate, &url, &url_len, &name, &name_len) == FAILURE) { return; @@ -1533,30 +1534,40 @@ PHP_FUNCTION(smbclient_getxattr) } if (xattr_size == 0) { - RETURN_EMPTY_STRING(); + /* since version 4.16.9 and 4.17.5 this means success :( + * so there is no way to compute the buffer size + * see https://bugzilla.samba.org/show_bug.cgi?id=14808 + */ + xattr_size = DEFAULT_BUFFER_SIZE; + do { + if (values) { + efree(values); + xattr_size *= 2; + } + values = emalloc(xattr_size + 1); + retsize = smbc_getxattr(state->ctx, url, name, values, xattr_size + 1); + } while (retsize < 0 && xattr_size < MAXIMUM_BUFFER_SIZE); + } else { + values = emalloc(xattr_size + 1); + retsize = smbc_getxattr(state->ctx, url, name, values, xattr_size + 1); } -#if PHP_MAJOR_VERSION >= 7 - svalues = zend_string_alloc(xattr_size, 0); - retsize = smbc_getxattr(state->ctx, url, name, ZSTR_VAL(svalues), xattr_size + 1); - if (retsize > xattr_size) { /* time-of-check, time-of-use error */ - retsize = xattr_size; - } else if (retsize < 0) { - zend_string_release(svalues); - goto fail; - } - RETURN_STR(svalues); -#else - values = emalloc(xattr_size + 1); - retsize = smbc_getxattr(state->ctx, url, name, values, xattr_size + 1); - if (retsize > xattr_size) { /* time-of-check, time-of-use error */ + if (retsize == 0) { /* success, since 4.16.9 and 4.17.5 */ + retsize = strlen(values); + } else if (retsize > xattr_size) { /* time-of-check, time-of-use error */ retsize = xattr_size; } else if (retsize < 0) { efree(values); goto fail; } - RETURN_STRINGL(values, retsize, 0); + /* realloc the string to its real size */ +#if PHP_MAJOR_VERSION >= 7 + RETVAL_STRINGL(values, retsize); +#else + RETVAL_STRINGL(values, retsize, 1); #endif + efree(values); + return; fail: hide_password(url, url_len); @@ -1565,7 +1576,12 @@ PHP_FUNCTION(smbclient_getxattr) case ENOMEM: php_error(E_WARNING, "Couldn't get xattr for %s: out of memory", url); break; case EPERM: php_error(E_WARNING, "Couldn't get xattr for %s: permission denied", url); break; case ENOTSUP: php_error(E_WARNING, "Couldn't get xattr for %s: file system does not support extended attributes", url); break; - default: php_error(E_WARNING, "Couldn't get xattr for %s: unknown error (%d)", url, errno); break; + default: + if (xattr_size == MAXIMUM_BUFFER_SIZE) { + php_error(E_WARNING, "Couldn't get xattr for %s: internal buffer is too small", url); break; + } else { + php_error(E_WARNING, "Couldn't get xattr for %s: unknown error (%d)", url, errno); break; + } } RETURN_FALSE; } From 78b519144521c22f917696500be77fb46d236813 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Mon, 3 Apr 2023 15:56:01 +0200 Subject: [PATCH 2/3] loop from 16K to 256M faster --- smbclient.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/smbclient.c b/smbclient.c index 64bb7d5..8186724 100644 --- a/smbclient.c +++ b/smbclient.c @@ -1503,9 +1503,9 @@ PHP_FUNCTION(smbclient_listxattr) } -/* loop from 4K to 16M */ -#define DEFAULT_BUFFER_SIZE (4 << 10) -#define MAXIMUM_BUFFER_SIZE (32 << 20) +/* loop from 16K to 256M */ +#define DEFAULT_BUFFER_SIZE (16 << 10) +#define MAXIMUM_BUFFER_SIZE (256 << 20) PHP_FUNCTION(smbclient_getxattr) { @@ -1542,7 +1542,7 @@ PHP_FUNCTION(smbclient_getxattr) do { if (values) { efree(values); - xattr_size *= 2; + xattr_size *= 4; } values = emalloc(xattr_size + 1); retsize = smbc_getxattr(state->ctx, url, name, values, xattr_size + 1); From a7b076a05b16047a09b5194782505c68d98208b7 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Mon, 3 Apr 2023 16:00:16 +0200 Subject: [PATCH 3/3] test that smbclient_getxattr returns not empty string --- tests/GetxattrTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/GetxattrTest.php b/tests/GetxattrTest.php index 539a0d0..266f353 100644 --- a/tests/GetxattrTest.php +++ b/tests/GetxattrTest.php @@ -8,7 +8,7 @@ public function $state = smbclient_state_new(); smbclient_state_init($state, null, SMB_USER, SMB_PASS); $attr = smbclient_getxattr($state, 'smb://'.SMB_HOST.'/'.SMB_SHARE.'/testdir/testfile.txt', 'system.*'); - $this->assertTrue(is_string($attr)); + $this->assertTrue(is_string($attr) && strlen($attr)); } public function @@ -17,7 +17,7 @@ public function $state = smbclient_state_new(); smbclient_state_init($state, null, SMB_USER, SMB_PASS); $attr = smbclient_getxattr($state, 'smb://'.SMB_HOST.'/'.SMB_SHARE.'/testdir', 'system.*'); - $this->assertTrue(is_string($attr)); + $this->assertTrue(is_string($attr) && strlen($attr)); } public function @@ -26,7 +26,7 @@ public function $state = smbclient_state_new(); smbclient_state_init($state, null, SMB_USER, SMB_PASS); $attr = smbclient_getxattr($state, 'smb://'.SMB_HOST.'/'.SMB_SHARE, 'system.*'); - $this->assertTrue(is_string($attr)); + $this->assertTrue(is_string($attr) && strlen($attr)); } /**