From c3da23209b45110fc637d96ba8f720597ccb5489 Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Tue, 20 Jun 2017 19:06:25 -0600 Subject: [PATCH] rktio: replace Windows invalid-decoding hack Report a proper error when path decoding fails, instead of synthesizing a path hat shouldn't exist. The rktio conversion made it much easier to report the error at the Racket level like other filesystem errors. --- racket/src/rktio/rktio.h | 1 + racket/src/rktio/rktio_envvars.c | 53 ++++++++++------ racket/src/rktio/rktio_error.c | 1 + racket/src/rktio/rktio_file.c | 13 +++- racket/src/rktio/rktio_fs.c | 103 +++++++++++++++++++++++++------ racket/src/rktio/rktio_private.h | 6 ++ racket/src/rktio/rktio_process.c | 22 ++++--- racket/src/rktio/rktio_wide.c | 62 +++++-------------- 8 files changed, 167 insertions(+), 94 deletions(-) diff --git a/racket/src/rktio/rktio.h b/racket/src/rktio/rktio.h index c2546b6e9f..dab22fea94 100644 --- a/racket/src/rktio/rktio.h +++ b/racket/src/rktio/rktio.h @@ -866,6 +866,7 @@ RKTIO_EXTERN int rktio_get_last_error(rktio_t *rktio); /* Error IDs of kind RKTIO_ERROR_KIND_RACKET */ enum { RKTIO_ERROR_UNSUPPORTED = 1, + RKTIO_ERROR_INVALID_PATH, /* Windows path-decoding failure */ RKTIO_ERROR_DOES_NOT_EXIST, RKTIO_ERROR_EXISTS, RKTIO_ERROR_ACCESS_DENIED, diff --git a/racket/src/rktio/rktio_envvars.c b/racket/src/rktio/rktio_envvars.c index bbfa8d06d2..d15bc51304 100644 --- a/racket/src/rktio/rktio_envvars.c +++ b/racket/src/rktio/rktio_envvars.c @@ -66,7 +66,10 @@ char *rktio_getenv(rktio_t *rktio, const char *name) #endif #ifdef RKTIO_SYSTEM_WINDOWS intptr_t value_size; - value_size = GetEnvironmentVariableW(WIDE_PATH_temp(name), NULL, 0); + wchar_t *wp; + wp = WIDE_PATH_temp(name); + if (!wp) return NULL; + value_size = GetEnvironmentVariableW(wp, NULL, 0); if (value_size) { wchar_t *value_w; char *value; @@ -120,14 +123,18 @@ int rktio_setenv(rktio_t *rktio, const char *name, const char *val) #endif #ifdef RKTIO_SYSTEM_WINDOWS int rc; - wchar_t *val_w; + wchar_t *val_w, *name_w; - if (val) - val_w = WIDE_PATH_copy(val); - else - val_w = NULL; + name_w = WIDE_PATH_temp(name); + if (!name_w) return 0; - rc = SetEnvironmentVariableW(WIDE_PATH_temp(name), val_w); + if (val) { + val_w = WIDE_PATH_copy(val); + if (!val_w) return 0; + } else + val_w = NULL; + + rc = SetEnvironmentVariableW(name_w, val_w); if (val_w) free(val_w); @@ -384,9 +391,14 @@ void *rktio_envvars_to_block(rktio_t *rktio, rktio_envvars_t *envvars) wchar_t *r, *s; for (i = 0; i < envvars->count; i++) { - len += wcslen(WIDE_PATH_temp(envvars->names[i])); - len += wcslen(WIDE_PATH_temp(envvars->vals[i])); - len += 2; + s = WIDE_PATH_temp(envvars->names[i]); + if (s) { + len += wcslen(s); + s = WIDE_PATH_temp(envvars->vals[i]); + if (s) + len += wcslen(s); + len += 2; + } } r = (wchar_t *)malloc((len + 1) * sizeof(wchar_t)); @@ -395,15 +407,18 @@ void *rktio_envvars_to_block(rktio_t *rktio, rktio_envvars_t *envvars) for (i = 0; i < envvars->count; i++) { s = WIDE_PATH_temp(envvars->names[i]); - slen = wcslen(s); - memcpy(r + len, s, slen * sizeof(wchar_t)); - len += slen; - r[len++] = '='; - s = WIDE_PATH_temp(envvars->vals[i]); - slen = wcslen(s); - memcpy(r + len, s, slen * sizeof(wchar_t)); - len += slen; - r[len++] = 0; + if (s) { + slen = wcslen(s); + memcpy(r + len, s, slen * sizeof(wchar_t)); + len += slen; + r[len++] = '='; + s = WIDE_PATH_temp(envvars->vals[i]); + if (!s) s = L""; + slen = wcslen(s); + memcpy(r + len, s, slen * sizeof(wchar_t)); + len += slen; + r[len++] = 0; + } } r[len] = 0; diff --git a/racket/src/rktio/rktio_error.c b/racket/src/rktio/rktio_error.c index 605f7b6b68..8c6f15e7ef 100644 --- a/racket/src/rktio/rktio_error.c +++ b/racket/src/rktio/rktio_error.c @@ -13,6 +13,7 @@ typedef struct err_str_t { err_str_t err_strs[] = { { RKTIO_ERROR_UNSUPPORTED, "unsupported" }, + { RKTIO_ERROR_INVALID_PATH, "invalid path encoding" }, { RKTIO_ERROR_DOES_NOT_EXIST, "no such file or directory" }, { RKTIO_ERROR_EXISTS, "file or directory already exists" }, { RKTIO_ERROR_LINK_FAILED, "link creation failed" }, diff --git a/racket/src/rktio/rktio_file.c b/racket/src/rktio/rktio_file.c index d1be1e3ef2..35da8ac260 100644 --- a/racket/src/rktio/rktio_file.c +++ b/racket/src/rktio/rktio_file.c @@ -64,8 +64,13 @@ static rktio_fd_t *open_read(rktio_t *rktio, const char *filename, int modes) #ifdef RKTIO_SYSTEM_WINDOWS HANDLE fd; rktio_fd_t *rfd; + wchar_t *wp; + + wp = WIDE_PATH_temp(filename); + if (!wp) + return NULL; - fd = CreateFileW(WIDE_PATH_temp(filename), + fd = CreateFileW(wp, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, @@ -166,6 +171,7 @@ static rktio_fd_t *open_write(rktio_t *rktio, const char *filename, int modes) HANDLE fd; int hmode; rktio_fd_t *rfd; + wchar_t *wp; if (modes & RKTIO_OPEN_MUST_EXIST) { if (modes & RKTIO_OPEN_TRUNCATE) @@ -177,7 +183,10 @@ static rktio_fd_t *open_write(rktio_t *rktio, const char *filename, int modes) else hmode = CREATE_NEW; - fd = CreateFileW(WIDE_PATH_temp(filename), + wp = WIDE_PATH_temp(filename); + if (!wp) return NULL; + + fd = CreateFileW(wp, GENERIC_WRITE | ((modes & RKTIO_OPEN_READ) ? GENERIC_READ : 0), FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, diff --git a/racket/src/rktio/rktio_fs.c b/racket/src/rktio/rktio_fs.c index 2c2838e2ca..eabc977a67 100644 --- a/racket/src/rktio/rktio_fs.c +++ b/racket/src/rktio/rktio_fs.c @@ -232,12 +232,19 @@ static char *UNC_readlink(rktio_t *rktio, const char *fn) mz_REPARSE_DATA_BUFFER *rp; int len, off; wchar_t *lk; + const wchar_t *wp; init_procs(); if (!DeviceIoControlProc) return NULL; - h = CreateFileW(WIDE_PATH_temp(fn), FILE_READ_ATTRIBUTES, + wp = WIDE_PATH_temp(fn); + if (!wp) { + /* Treat invalid path as non-existent path */ + return MSC_IZE(strdup)(fn); + } + + h = CreateFileW(wp, FILE_READ_ATTRIBUTES, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | mzFILE_FLAG_OPEN_REPARSE_POINT, @@ -342,6 +349,7 @@ static int UNC_stat(rktio_t *rktio, const char *dirname, int *flags, int *isdir, char *copy; WIN32_FILE_ATTRIBUTE_DATA fad; int len, must_be_dir = 0; + const wchar_t *wp; if (resolved_path) *resolved_path = NULL; @@ -400,7 +408,14 @@ static int UNC_stat(rktio_t *rktio, const char *dirname, int *flags, int *isdir, copy[4] = 0; } - if (!GetFileAttributesExW(WIDE_PATH_temp(copy), GetFileExInfoStandard, &fad)) { + wp = WIDE_PATH_temp(copy); + if (!wp) { + /* Treat invalid path as non-existent */ + free(copy); + return 0; + } + + if (!GetFileAttributesExW(wp, GetFileExInfoStandard, &fad)) { get_windows_error(); free(copy); return 0; @@ -562,9 +577,13 @@ int rktio_is_regular_file(rktio_t *rktio, const char *filename) return 0; # else struct MSC_IZE(stat) buf; + const WIDE_PATH_t *wp; + + wp = MSC_WIDE_PATH_temp(filename); + if (!wp) return 0; while (1) { - if (!MSC_W_IZE(stat)(MSC_WIDE_PATH_temp(filename), &buf)) + if (!MSC_W_IZE(stat)(wp, &buf)) break; else if (errno != EINTR) return 0; @@ -651,12 +670,16 @@ char *rktio_get_current_directory(rktio_t *rktio) #endif } -int rktio_set_current_directory(rktio_t *rktio, const char *path) +rktio_ok_t rktio_set_current_directory(rktio_t *rktio, const char *path) { int err; + const WIDE_PATH_t *wp; + + wp = MSC_WIDE_PATH_temp(path); + if (!wp) return 0; while (1) { - err = MSC_W_IZE(chdir)(MSC_WIDE_PATH_temp(path)); + err = MSC_W_IZE(chdir)(wp); if (!err || (errno != EINTR)) break; } @@ -706,7 +729,10 @@ static rktio_identity_t *get_identity(rktio_t *rktio, rktio_fd_t *fd, const char init_procs(); if (path) { - fdh = CreateFileW(WIDE_PATH_temp(path), + const wchar_t *wp; + wp = WIDE_PATH_temp(path); + if (!wp) return 0; + fdh = CreateFileW(wp, 0, /* not even read access => just get info */ FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, @@ -779,8 +805,14 @@ int rktio_delete_file(rktio_t *rktio, const char *fn, int enable_write_on_fail) { #ifdef RKTIO_SYSTEM_WINDOWS int errid; - if (DeleteFileW(WIDE_PATH_temp(fn))) + const wchar_t *wp; + + wp = WIDE_PATH_temp(fn); + if (!wp) return 0; + + if (DeleteFileW(wp)) return 1; + errid = GetLastError(); if ((errid == ERROR_ACCESS_DENIED) && enable_write_on_fail) { /* Maybe it's just that the file has no write permission. Provide a more @@ -810,13 +842,20 @@ int rktio_rename_file(rktio_t *rktio, const char *dest, const char *src, int exi { #ifdef RKTIO_SYSTEM_WINDOWS int errid; - wchar_t *src_w = WIDE_PATH_copy(src); - - if (MoveFileExW(src_w, WIDE_PATH_temp(dest), (exists_ok ? MOVEFILE_REPLACE_EXISTING : 0))) { + wchar_t *src_w; + const wchar_t *dest_w; + + src_w = WIDE_PATH_copy(src); + if (!src_w) return 0; + + dest_w = WIDE_PATH_temp(dest); + if (!dest_w) return 0; + + if (MoveFileExW(src_w, dest_w, (exists_ok ? MOVEFILE_REPLACE_EXISTING : 0))) { free(src_w); return 1; } - + errid = GetLastError(); if (errid == ERROR_CALL_NOT_IMPLEMENTED) { @@ -906,6 +945,7 @@ int rktio_make_directory(rktio_t *rktio, const char *filename) #else int len; char *copied = NULL; + const WIDE_PATH_t *wp; /* Make sure path doesn't have trailing separator: */ len = strlen(filename); @@ -916,7 +956,9 @@ int rktio_make_directory(rktio_t *rktio, const char *filename) } while (1) { - if (!MSC_W_IZE(mkdir)(MSC_WIDE_PATH_temp(filename) + wp = MSC_WIDE_PATH_temp(filename); + if (!wp) return 0; + if (!MSC_W_IZE(mkdir)(wp # ifndef MKDIR_NO_MODE_FLAG , 0777 # endif @@ -942,9 +984,12 @@ int rktio_delete_directory(rktio_t *rktio, const char *filename, const char *cur #ifdef RKTIO_SYSTEM_WINDOWS int tried_cwd = 0, tried_perm = 0; #endif + const WIDE_PATH_t *wp; while (1) { - if (!MSC_W_IZE(rmdir)(MSC_WIDE_PATH_temp(filename))) + wp = MSC_WIDE_PATH_temp(filename); + if (!wp) return 0; + if (!MSC_W_IZE(rmdir)(wp)) return 1; # ifdef RKTIO_SYSTEM_WINDOWS else if ((errno == EACCES) && !tried_cwd) { @@ -975,14 +1020,21 @@ int rktio_make_link(rktio_t *rktio, const char *src, const char *dest, int dest_ if (CreateSymbolicLinkProc) { int flags; - wchar_t *src_w = WIDE_PATH_copy(src); + wchar_t *src_w; + wchar_t *dest_w; if (dest_is_directory) flags = 0x1; /* directory */ else flags = 0; /* file */ - if (CreateSymbolicLinkProc(src_w, WIDE_PATH_temp(dest), flags)) { + src_w = WIDE_PATH_copy(src); + if (!src_w) return 0; + + dest_w = WIDE_PATH_temp(dest); + if (!dest_w) return 0; + + if (CreateSymbolicLinkProc(src_w, dest_w, flags)) { free(src_w); return 1; } @@ -1034,9 +1086,12 @@ int rktio_set_file_modify_seconds(rktio_t *rktio, const char *file, rktio_timest { while (1) { struct MSC_IZE(utimbuf) ut; + const WIDE_PATH_t *wp; ut.actime = secs; ut.modtime = secs; - if (!MSC_W_IZE(utime)(MSC_WIDE_PATH_temp(file), &ut)) + wp = MSC_WIDE_PATH_temp(file); + if (!wp) return 0; + if (!MSC_W_IZE(utime)(wp, &ut)) return 1; if (errno != EINTR) break; @@ -1329,6 +1384,7 @@ rktio_directory_list_t *rktio_directory_list_start(rktio_t *rktio, const char *f FF_HANDLE_TYPE hfile; FF_TYPE info; rktio_directory_list_t *dl; + const wchar_t *wp; retry: @@ -1372,7 +1428,10 @@ rktio_directory_list_t *rktio_directory_list_start(rktio_t *rktio, const char *f memcpy(pattern + len, "*.*", 4); } - hfile = FIND_FIRST(WIDE_PATH_temp(pattern), &info); + wp = WIDE_PATH_temp(pattern); + if (!wp) return NULL; + + hfile = FIND_FIRST(wp, &info); if (FIND_FAILED(hfile)) { int err_val; err_val = GetLastError(); @@ -1563,9 +1622,15 @@ rktio_file_copy_t *rktio_copy_file_start(rktio_t *rktio, const char *dest, const #endif #ifdef RKTIO_SYSTEM_WINDOWS int err_val = 0; - wchar_t *src_w = WIDE_PATH_copy(src); + wchar_t *src_w; + const wchar_t *dest_w; - if (CopyFileW(src_w, WIDE_PATH_temp(dest), !exists_ok)) { + src_w = WIDE_PATH_copy(src); + if (!src_w) return NULL; + dest_w = WIDE_PATH_temp(dest); + if (!dest_w) return NULL; + + if (CopyFileW(src_w, dest_w, !exists_ok)) { rktio_file_copy_t *fc; free(src_w); /* Return a pointer to indicate success: */ diff --git a/racket/src/rktio/rktio_private.h b/racket/src/rktio/rktio_private.h index d2ef62e137..893ad825f5 100644 --- a/racket/src/rktio/rktio_private.h +++ b/racket/src/rktio/rktio_private.h @@ -226,6 +226,12 @@ char *rktio_convert_from_wchar(const wchar_t *ws, int free_given); # define NARROW_PATH_copy(ws) rktio_convert_from_wchar(ws, 0) # define NARROW_PATH_copy_then_free(ws) rktio_convert_from_wchar(ws, 1) +typedef wchar_t WIDE_PATH_t; + +#else + +typedef char WIDE_PATH_t; + #endif /*========================================================================*/ diff --git a/racket/src/rktio/rktio_process.c b/racket/src/rktio/rktio_process.c index f1a79e9d5d..1a1bc20067 100644 --- a/racket/src/rktio/rktio_process.c +++ b/racket/src/rktio/rktio_process.c @@ -954,8 +954,8 @@ void rktio_process_forget(rktio_t *rktio, rktio_process_t *sp) int rktio_process_init(rktio_t *rktio) { #if defined(CENTRALIZED_SIGCHILD) - /* Block SIGCHLD as earyl as possible, because it's a per-thread - setting on Linux, and we want SIGCHLD blocked everywhere. */ + /* Block SIGCHLD as early as possible, because + it's a per-thread setting on Linux, and we want SIGCHLD blocked everywhere. */ block_sigchld(); centralized_start_child_signal_handler(); @@ -1050,7 +1050,7 @@ static intptr_t do_spawnv(rktio_t *rktio, int use_jo; intptr_t cr_flag; char *cmdline; - wchar_t *cmdline_w, *wd_w; + wchar_t *cmdline_w, *wd_w, *command_w; STARTUPINFOW startup; PROCESS_INFORMATION info; @@ -1116,11 +1116,15 @@ static intptr_t do_spawnv(rktio_t *rktio, if (!exact_cmdline) free(cmdline); wd_w = WIDE_PATH_copy(wd); + command_w = WIDE_PATH_temp(command); - if (CreateProcessW(WIDE_PATH_temp(command), cmdline_w, - NULL, NULL, 1 /*inherit*/, - cr_flag, env, wd_w, - &startup, &info)) { + if (cmdline_w + && wd_w + && command_w + && CreateProcessW(command_w, cmdline_w, + NULL, NULL, 1 /*inherit*/, + cr_flag, env, wd_w, + &startup, &info)) { if (use_jo) AssignProcessToJobObject(rktio->process_job_object, info.hProcess); CloseHandle(info.hThread); @@ -1129,8 +1133,8 @@ static intptr_t do_spawnv(rktio_t *rktio, free(wd_w); return (intptr_t)info.hProcess; } else { - free(cmdline_w); - free(wd_w); + if (cmdline_w) free(cmdline_w); + if (wd_w) free(wd_w); return -1; } } diff --git a/racket/src/rktio/rktio_wide.c b/racket/src/rktio/rktio_wide.c index 53d07ed006..9be43f6b01 100644 --- a/racket/src/rktio/rktio_wide.c +++ b/racket/src/rktio/rktio_wide.c @@ -26,7 +26,7 @@ void rktio_init_wide(rktio_t *rktio) /* A UTF-8 to UTF-16 conversion, but accepts an extended variant of UTF-16 that accommodates unpaired surrogates, so that all 16-byte sequences are accessible. */ -static intptr_t utf8ish_to_utf16ish(const unsigned char *s, intptr_t end, unsigned short *us, int replacement) +static intptr_t utf8ish_to_utf16ish(const unsigned char *s, intptr_t end, unsigned short *us) { intptr_t i, j, oki; int state; @@ -48,14 +48,9 @@ static intptr_t utf8ish_to_utf16ish(const unsigned char *s, intptr_t end, unsign if (sc < 0x80) { if (state) { /* In a sequence, but didn't continue */ - state = 0; - nextbits = 0; - v = replacement; - i = oki; - j += init_doki; - } else { - v = sc; + return -1; } + v = sc; } else if ((sc & 0xC0) == 0x80) { /* Continues a sequence ... */ if (state) { @@ -72,28 +67,19 @@ static intptr_t utf8ish_to_utf16ish(const unsigned char *s, intptr_t end, unsign /* We finished. One last check: */ if (v > 0x10FFFF) { /* illegal code units */ - v = replacement; - j += init_doki; - i = oki; + return -1; } } else { /* ... but we're missing required bits. */ - state = 0; - nextbits = 0; - v = replacement; - j += init_doki; - i = oki; + return -1; } } else { /* ... but we're not in one */ - v = replacement; + return -1; } } else if (state) { /* bad: already in a sequence */ - state = 0; - v = replacement; - i = oki; - j += init_doki; + return -1; } else { if ((sc & 0xE0) == 0xC0) { if (sc & 0x1E) { @@ -122,7 +108,7 @@ static intptr_t utf8ish_to_utf16ish(const unsigned char *s, intptr_t end, unsign /* Else will be larger than 0x10FFFF, so fail */ } /* Too small, or 0xFF or 0xFe, or start of a 5- or 6-byte sequence */ - v = replacement; + return -1; } /* If we get here, we're supposed to output v */ @@ -149,18 +135,7 @@ static intptr_t utf8ish_to_utf16ish(const unsigned char *s, intptr_t end, unsign if ((v >= 0xD800) && (v <= 0xDFFF)) { if (pending_surrogate && ((v & 0xDC00) == 0xDC00)) { /* This would look like a surrogate pair... */ - /* We need to fill in 6 replacement substitutions, - one for each input byte. If we can't put all 6, - then don't use any input. */ - int p; - if (us) { - for (p = 0; p < 5; p++) { - us[j+p] = replacement; - } - } - j += 5; - v = replacement; - pending_surrogate = 0; + return -1; } else { if (pending_surrogate) { if (us) @@ -199,13 +174,8 @@ static intptr_t utf8ish_to_utf16ish(const unsigned char *s, intptr_t end, unsign j++; } - if (state) { - for (i = oki; i < end; i++) { - if (us) - us[j] = replacement; - j++; - } - } + if (state) + return -1; return j; } @@ -286,15 +256,17 @@ static intptr_t utf16ish_to_utf8ish(const unsigned short *us, intptr_t end, unsi #define RKTIO_MAX_IDEAL_BUFFER_SIZE 4096 wchar_t *rktio_convert_to_wchar(rktio_t *rktio, const char *s, int do_copy) -/* This function uses '\t' in place of invalid UTF-8 encoding - bytes, because '\t' is not a legal filename under Windows. */ { intptr_t len, l; wchar_t *ws; l = strlen(s)+1; /* add nul terminator */ - len = utf8ish_to_utf16ish((unsigned char *)s, l, NULL, '\t'); + len = utf8ish_to_utf16ish((unsigned char *)s, l, NULL); + if (len < 0) { + set_racket_error(RKTIO_ERROR_INVALID_PATH); + return NULL; + } if (do_copy) ws = malloc(sizeof(wchar_t) * len); @@ -312,7 +284,7 @@ wchar_t *rktio_convert_to_wchar(rktio_t *rktio, const char *s, int do_copy) } else ws = rktio->wide_buffer; - (void)utf8ish_to_utf16ish((unsigned char *)s, l, (unsigned short*)ws, '\t'); + (void)utf8ish_to_utf16ish((unsigned char *)s, l, (unsigned short*)ws); return ws; }