Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Zend: Refactor virtual_rename() to take path lengths
  • Loading branch information
Girgias committed Aug 23, 2024
commit 64a2693c74df87a8cc74b46c45f6919705fcf996
25 changes: 10 additions & 15 deletions Zend/zend_virtual_cwd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1501,36 +1501,31 @@ CWD_API int virtual_creat(const char *path, mode_t mode) /* {{{ */
}
/* }}} */

CWD_API int virtual_rename(const char *oldname, const char *newname) /* {{{ */
CWD_API zend_result virtual_rename(const char *old_name, size_t old_name_len, const char *new_name, size_t new_name_len) /* {{{ */
{
cwd_state old_state;
cwd_state new_state;
size_t old_name_length = strlen(oldname);
size_t new_name_length = strlen(newname);
int retval;

CWD_STATE_COPY(&old_state, &CWDG(cwd));
if (virtual_file_ex(&old_state, oldname, old_name_length, NULL, CWD_EXPAND)) {
if (virtual_file_ex(&old_state, old_name, old_name_len, NULL, CWD_EXPAND)) {
CWD_STATE_FREE_ERR(&old_state);
return -1;
return FAILURE;
}
oldname = old_state.cwd;

CWD_STATE_COPY(&new_state, &CWDG(cwd));
if (virtual_file_ex(&new_state, newname, new_name_length, NULL, CWD_EXPAND)) {
if (virtual_file_ex(&new_state, new_name, new_name_len, NULL, CWD_EXPAND)) {
CWD_STATE_FREE_ERR(&old_state);
CWD_STATE_FREE_ERR(&new_state);
return -1;
return FAILURE;
}
newname = new_state.cwd;

/* rename on windows will fail if newname already exists.
MoveFileEx has to be used */
zend_result retval;
#ifdef ZEND_WIN32
/* MoveFileEx returns 0 on failure, other way 'round for this function */
retval = php_win32_ioutil_rename(oldname, newname);
/* rename on windows will fail if new_name already exists. MoveFileEx has to be used */
/* MoveFileEx returns 0 on failure, other way round for this function */
retval = php_win32_ioutil_rename(old_state.cwd, old_state.cwd_length, new_name, new_name_len);
#else
retval = rename(oldname, newname);
retval = virtual_rename_native(old_state.cwd, old_state.cwd_length, new_state.cwd, new_state.cwd_length);
#endif

CWD_STATE_FREE_ERR(&old_state);
Expand Down
16 changes: 11 additions & 5 deletions Zend/zend_virtual_cwd.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ CWD_API char *virtual_realpath(const char *path, char *real_path);
CWD_API FILE *virtual_fopen(const char *path, const char *mode);
CWD_API int virtual_open(const char *path, int flags, ...);
CWD_API int virtual_creat(const char *path, mode_t mode);
CWD_API int virtual_rename(const char *oldname, const char *newname);
CWD_API zend_result virtual_rename(const char *old_name, size_t old_name_len, const char *new_name, size_t new_name_len);
CWD_API int virtual_stat(const char *path, zend_stat_t *buf);
CWD_API int virtual_lstat(const char *path, zend_stat_t *buf);
CWD_API int virtual_unlink(const char *path);
Expand Down Expand Up @@ -254,6 +254,12 @@ CWD_API zend_long realpath_cache_size(void);
CWD_API zend_long realpath_cache_max_buckets(void);
CWD_API realpath_cache_bucket** realpath_cache_get_buckets(void);


static zend_always_inline zend_result virtual_rename_native(const char *old_name,
ZEND_ATTRIBUTE_UNUSED size_t old_name_len, const char *new_name, ZEND_ATTRIBUTE_UNUSED size_t new_name_len) {
return (rename(old_name, new_name) == 0) ? SUCCESS : FAILURE;
}

#ifdef CWD_EXPORTS
extern void virtual_cwd_main_cwd_init(uint8_t);
#endif
Expand All @@ -275,7 +281,7 @@ extern void virtual_cwd_main_cwd_init(uint8_t);
#define VCWD_CHDIR_FILE(path, path_len) virtual_chdir_file(path, path_len, (int (*)(const char *)) virtual_chdir)
#define VCWD_GETWD(buf)
#define VCWD_REALPATH(path, real_path) virtual_realpath(path, real_path)
#define VCWD_RENAME(oldname, newname) virtual_rename(oldname, newname)
#define VCWD_RENAME(old_name, old_name_length, new_name, new_name_length) virtual_rename(old_name, old_name_length, new_name, new_name_length)
#define VCWD_STAT(path, buff) virtual_stat(path, buff)
# define VCWD_LSTAT(path, buff) virtual_lstat(path, buff)
#define VCWD_UNLINK(path) virtual_unlink(path)
Expand Down Expand Up @@ -304,7 +310,7 @@ extern void virtual_cwd_main_cwd_init(uint8_t);
#define VCWD_FOPEN(path, mode) php_win32_ioutil_fopen(path, mode)
#define VCWD_OPEN(path, flags) php_win32_ioutil_open(path, flags)
#define VCWD_OPEN_MODE(path, flags, mode) php_win32_ioutil_open(path, flags, mode)
# define VCWD_RENAME(oldname, newname) php_win32_ioutil_rename(oldname, newname)
#define VCWD_RENAME(old_name, old_name_length, new_name, new_name_length) php_win32_ioutil_rename(old_name, old_name_length, new_name, new_name_length)
#define VCWD_MKDIR(pathname, mode) php_win32_ioutil_mkdir(pathname, mode)
#define VCWD_RMDIR(pathname) php_win32_ioutil_rmdir(pathname)
#define VCWD_UNLINK(path) php_win32_ioutil_unlink(path)
Expand All @@ -316,8 +322,8 @@ extern void virtual_cwd_main_cwd_init(uint8_t);
#define VCWD_FOPEN(path, mode) fopen(path, mode)
#define VCWD_OPEN(path, flags) open(path, flags)
#define VCWD_OPEN_MODE(path, flags, mode) open(path, flags, mode)
# define VCWD_RENAME(oldname, newname) rename(oldname, newname)
#define VCWD_MKDIR(pathname, mode) mkdir(pathname, mode)
#define VCWD_RENAME(old_name, old_name_len, new_name, new_name_len) virtual_rename_native(old_name, old_name_len, new_name, new_name_len)
#define VCWD_MKDIR(path, mode) mkdir(path, mode)
#define VCWD_RMDIR(pathname) rmdir(pathname)
#define VCWD_UNLINK(path) unlink(path)
#define VCWD_CHDIR(path) chdir(path)
Expand Down
7 changes: 4 additions & 3 deletions ext/soap/php_sdl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2088,7 +2088,7 @@ static void sdl_serialize_soap_body(const sdlSoapBindingFunctionBodyPtr body, co
}
}

static void add_sdl_to_cache(const char *fn, const char *uri, time_t t, sdlPtr sdl)
static void add_sdl_to_cache(const char *fn, size_t fn_len, const char *uri, time_t t, sdlPtr sdl)
{
smart_str buf = {0};
smart_str *out = &buf;
Expand Down Expand Up @@ -2363,7 +2363,7 @@ static void add_sdl_to_cache(const char *fn, const char *uri, time_t t, sdlPtr s
/* Make sure that incomplete files (e.g. due to disk space issues, see bug #66150) are not utilised. */
if (valid_file) {
/* This is allowed to fail, this means that another process was raced to create the file. */
if (VCWD_RENAME(ZSTR_VAL(temp_file_path), fn) < 0) {
if (VCWD_RENAME(ZSTR_VAL(temp_file_path), ZSTR_LEN(temp_file_path), fn, fn_len) < 0) {
VCWD_UNLINK(ZSTR_VAL(temp_file_path));
}
}
Expand Down Expand Up @@ -3329,7 +3329,8 @@ sdlPtr get_sdl(zval *this_ptr, char *uri, zend_long cache_wsdl)

if ((cache_wsdl & WSDL_CACHE_DISK) && key) {
if (sdl) {
add_sdl_to_cache(key, uri, t, sdl);
// TODO Is it possible to know key_len before?
add_sdl_to_cache(key, strlen(key), uri, t, sdl);
}
efree(key);
}
Expand Down
2 changes: 1 addition & 1 deletion ext/standard/basic_functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -2379,7 +2379,7 @@ PHP_FUNCTION(move_uploaded_file)
RETURN_FALSE;
}

if (VCWD_RENAME(path, new_path) == 0) {
if (VCWD_RENAME(path, path_len, new_path, new_path_len) == 0) {
successful = 1;
#ifndef PHP_WIN32
oldmask = umask(077);
Expand Down
10 changes: 7 additions & 3 deletions main/streams/plain_wrapper.c
Original file line number Diff line number Diff line change
Expand Up @@ -1274,30 +1274,34 @@ static int php_plain_files_rename(php_stream_wrapper *wrapper, const char *url_f
return 0;
}

size_t url_from_len = strlen(url_from);
size_t url_to_len = strlen(url_to);
#ifdef PHP_WIN32
if (!php_win32_check_trailing_space(url_from, strlen(url_from))) {
if (!php_win32_check_trailing_space(url_from, url_from_len)) {
php_win32_docref2_from_error(ERROR_INVALID_NAME, url_from, url_to);
return 0;
}
if (!php_win32_check_trailing_space(url_to, strlen(url_to))) {
if (!php_win32_check_trailing_space(url_to, url_to_len)) {
php_win32_docref2_from_error(ERROR_INVALID_NAME, url_from, url_to);
return 0;
}
#endif

if (strncasecmp(url_from, "file://", sizeof("file://") - 1) == 0) {
url_from += sizeof("file://") - 1;
url_from_len -= sizeof("file://");
}

if (strncasecmp(url_to, "file://", sizeof("file://") - 1) == 0) {
url_to += sizeof("file://") - 1;
url_to_len -= sizeof("file://");
}

if (php_check_open_basedir(url_from) || php_check_open_basedir(url_to)) {
return 0;
}

ret = VCWD_RENAME(url_from, url_to);
ret = VCWD_RENAME(url_from, url_from_len, url_to, url_to_len);

if (ret == -1) {
#ifndef PHP_WIN32
Expand Down
13 changes: 5 additions & 8 deletions win32/ioutil.c
Original file line number Diff line number Diff line change
Expand Up @@ -464,21 +464,18 @@ PW32IO int php_win32_ioutil_chdir_w(const wchar_t *path)
return ret;
}/*}}}*/

PW32IO int php_win32_ioutil_rename_w(const wchar_t *oldname, const wchar_t *newname)
PW32IO zend_result php_win32_ioutil_rename_w(const wchar_t *oldname, const wchar_t *newname)
{/*{{{*/
int ret = 0;

PHP_WIN32_IOUTIL_CHECK_PATH_W(oldname, -1, 0)
PHP_WIN32_IOUTIL_CHECK_PATH_W(newname, -1, 0)

PHP_WIN32_IOUTIL_CHECK_PATH_W(oldname, FAILURE, 0)
PHP_WIN32_IOUTIL_CHECK_PATH_W(newname, FAILURE, 0)

if (!MoveFileExW(oldname, newname, MOVEFILE_REPLACE_EXISTING|MOVEFILE_COPY_ALLOWED)) {
DWORD err = GetLastError();
ret = -1;
SET_ERRNO_FROM_WIN32_CODE(err);
return FAILURE;
}

return ret;
return SUCCESS;
}/*}}}*/

PW32IO wchar_t *php_win32_ioutil_getcwd_w(wchar_t *buf, size_t len)
Expand Down
29 changes: 12 additions & 17 deletions win32/ioutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ PW32IO size_t php_win32_ioutil_dirname(char *buf, size_t len);

PW32IO int php_win32_ioutil_open_w(const wchar_t *path, int flags, ...);
PW32IO int php_win32_ioutil_chdir_w(const wchar_t *path);
PW32IO int php_win32_ioutil_rename_w(const wchar_t *oldname, const wchar_t *newname);
PW32IO zend_result php_win32_ioutil_rename_w(const wchar_t *oldname, const wchar_t *newname);
PW32IO wchar_t *php_win32_ioutil_getcwd_w(wchar_t *buf, size_t len);
PW32IO int php_win32_ioutil_unlink_w(const wchar_t *path);
PW32IO int php_win32_ioutil_access_w(const wchar_t *path, mode_t mode);
Expand Down Expand Up @@ -418,47 +418,42 @@ __forceinline static FILE *php_win32_ioutil_fopen(const char *patha, const char
return ret;
}/*}}}*/

__forceinline static int php_win32_ioutil_rename(const char *oldnamea, const char *newnamea)
__forceinline static zend_result php_win32_ioutil_rename(const char *old_name_a, size_t old_name_a_len, const char *new_name_a, size_t new_name_a_len)
{/*{{{*/
wchar_t *oldnamew;
wchar_t *newnamew;
int ret;
DWORD err = 0;

oldnamew = php_win32_ioutil_any_to_w(oldnamea);
oldnamew = php_win32_ioutil_conv_any_to_w(old_name_a, old_name_a_len, PHP_WIN32_CP_IGNORE_LEN_P);
Comment on lines -428 to +419
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmb69 as don't have a Windows machine and can't test it, is such a function call change even sensible or not?
And if yes, can I also provide a size_t pointer for the 3rd argument so that PHP_WIN32_IOUTIL_CHECK_PATH_W() doesn't need to make a call to wstrlen()?

As seemingly this changes the behaviour of some tests in ways that I don't totally understand.

if (!oldnamew) {
SET_ERRNO_FROM_WIN32_CODE(ERROR_INVALID_PARAMETER);
return -1;
return FAILURE;
}
PHP_WIN32_IOUTIL_CHECK_PATH_W(oldnamew, -1, 1)
PHP_WIN32_IOUTIL_CHECK_PATH_W(oldnamew, FAILURE, 1)

newnamew = php_win32_ioutil_any_to_w(newnamea);
newnamew = php_win32_ioutil_conv_any_to_w(new_name_a, new_name_a_len, PHP_WIN32_CP_IGNORE_LEN_P);
if (!newnamew) {
free(oldnamew);
SET_ERRNO_FROM_WIN32_CODE(ERROR_INVALID_PARAMETER);
return -1;
return FAILURE;
} else {
size_t newnamew_len = wcslen(newnamew);
if (!PHP_WIN32_IOUTIL_PATH_IS_OK_W(newnamew, newnamew_len)) {
free(oldnamew);
free(newnamew);
SET_ERRNO_FROM_WIN32_CODE(ERROR_ACCESS_DENIED);
return -1;
return FAILURE;
}
}

ret = php_win32_ioutil_rename_w(oldnamew, newnamew);
if (0 > ret) {
err = GetLastError();
zend_result ret = php_win32_ioutil_rename_w(oldnamew, newnamew);
if (ret == FAILURE) {
DWORD err = GetLastError();
SET_ERRNO_FROM_WIN32_CODE(err);
}

free(oldnamew);
free(newnamew);

if (0 > ret) {
SET_ERRNO_FROM_WIN32_CODE(err);
}

return ret;
}/*}}}*/

Expand Down