From 4c0a9f579d3064f086b42a2d39aaea721e7e71ca Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Tue, 26 Sep 2023 16:05:42 -0700 Subject: [PATCH] Add error codes for file read/write/open failures (#1061) --- include/aws/common/error.h | 11 +++++ include/aws/common/file.h | 2 + source/common.c | 23 +++++++---- source/error.c | 16 ++++++-- source/file.c | 83 +++++++++++++++++++++----------------- source/log_writer.c | 4 +- source/logging.c | 4 +- source/posix/file.c | 4 +- source/windows/file.c | 4 +- 9 files changed, 96 insertions(+), 55 deletions(-) diff --git a/include/aws/common/error.h b/include/aws/common/error.h index 2c103d763..a40b193fa 100644 --- a/include/aws/common/error.h +++ b/include/aws/common/error.h @@ -130,6 +130,14 @@ void aws_register_error_info(const struct aws_error_info_list *error_info); AWS_COMMON_API void aws_unregister_error_info(const struct aws_error_info_list *error_info); +/** + * Convert a c library io error into an aws error, and raise it. + * If no conversion is found, fallback_aws_error_code is raised. + * Always returns AWS_OP_ERR. + */ +AWS_COMMON_API +int aws_translate_and_raise_io_error_or(int error_no, int fallback_aws_error_code); + /** * Convert a c library io error into an aws error, and raise it. * If no conversion is found, AWS_ERROR_SYS_CALL_FAILURE is raised. @@ -202,6 +210,9 @@ enum aws_common_error { AWS_ERROR_INVALID_UTF8, AWS_ERROR_GET_HOME_DIRECTORY_FAILED, AWS_ERROR_INVALID_XML, + AWS_ERROR_FILE_OPEN_FAILURE, + AWS_ERROR_FILE_READ_FAILURE, + AWS_ERROR_FILE_WRITE_FAILURE, AWS_ERROR_END_COMMON_RANGE = AWS_ERROR_ENUM_END_RANGE(AWS_C_COMMON_PACKAGE_ID) }; diff --git a/include/aws/common/file.h b/include/aws/common/file.h index 842b777d8..23937e866 100644 --- a/include/aws/common/file.h +++ b/include/aws/common/file.h @@ -191,6 +191,8 @@ bool aws_path_exists(const struct aws_string *path); * fseeko() on linux * * whence can either be SEEK_SET or SEEK_END + * + * Returns AWS_OP_SUCCESS, or AWS_OP_ERR (after an error has been raised). */ AWS_COMMON_API int aws_fseek(FILE *file, int64_t offset, int whence); diff --git a/source/common.c b/source/common.c index 79e301539..9a6f56cf4 100644 --- a/source/common.c +++ b/source/common.c @@ -220,7 +220,7 @@ static struct aws_error_info errors[] = { ), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_SYS_CALL_FAILURE, - "System call failure"), + "System call failure."), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_FILE_INVALID_PATH, "Invalid file path."), @@ -232,7 +232,7 @@ static struct aws_error_info errors[] = { "User does not have permission to perform the requested action."), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_STREAM_UNSEEKABLE, - "Stream does not support seek operations"), + "Stream does not support seek operations."), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_C_STRING_BUFFER_NOT_NULL_TERMINATED, "A c-string like buffer was passed but a null terminator was not found within the bounds of the buffer."), @@ -244,7 +244,7 @@ static struct aws_error_info errors[] = { "Attempt to divide a number by zero."), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_INVALID_FILE_HANDLE, - "Invalid file handle"), + "Invalid file handle."), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_OPERATION_INTERUPTED, "The operation was interrupted." @@ -255,16 +255,25 @@ static struct aws_error_info errors[] = { ), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_PLATFORM_NOT_SUPPORTED, - "Feature not supported on this platform"), + "Feature not supported on this platform."), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_INVALID_UTF8, - "Invalid UTF-8"), + "Invalid UTF-8."), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_GET_HOME_DIRECTORY_FAILED, - "Failed to get home directory"), + "Failed to get home directory."), AWS_DEFINE_ERROR_INFO_COMMON( AWS_ERROR_INVALID_XML, - "Invalid XML document"), + "Invalid XML document."), + AWS_DEFINE_ERROR_INFO_COMMON( + AWS_ERROR_FILE_OPEN_FAILURE, + "Failed opening file."), + AWS_DEFINE_ERROR_INFO_COMMON( + AWS_ERROR_FILE_READ_FAILURE, + "Failed reading from file."), + AWS_DEFINE_ERROR_INFO_COMMON( + AWS_ERROR_FILE_WRITE_FAILURE, + "Failed writing to file."), }; /* clang-format on */ diff --git a/source/error.c b/source/error.c index ad3cec869..239b318bd 100644 --- a/source/error.c +++ b/source/error.c @@ -194,11 +194,19 @@ void aws_unregister_error_info(const struct aws_error_info_list *error_info) { } int aws_translate_and_raise_io_error(int error_no) { + return aws_translate_and_raise_io_error_or(error_no, AWS_ERROR_SYS_CALL_FAILURE); +} + +int aws_translate_and_raise_io_error_or(int error_no, int fallback_aws_error_code) { switch (error_no) { case EINVAL: - return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT); - case ESPIPE: - return aws_raise_error(AWS_ERROR_STREAM_UNSEEKABLE); + /* If useful fallback code provided, raise that instead of AWS_ERROR_INVALID_ARGUMENT, + * which isn't very useful when it bubbles out from deep within some complex system. */ + if (fallback_aws_error_code != AWS_ERROR_SYS_CALL_FAILURE) { + return aws_raise_error(fallback_aws_error_code); + } else { + return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT); + } case EPERM: case EACCES: return aws_raise_error(AWS_ERROR_NO_PERMISSION); @@ -217,6 +225,6 @@ int aws_translate_and_raise_io_error(int error_no) { case ENOTEMPTY: return aws_raise_error(AWS_ERROR_DIRECTORY_NOT_EMPTY); default: - return aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); + return aws_raise_error(fallback_aws_error_code); } } diff --git a/source/file.c b/source/file.c index 01eb0a6af..00723555f 100644 --- a/source/file.c +++ b/source/file.c @@ -38,48 +38,59 @@ int aws_byte_buf_init_from_file(struct aws_byte_buf *out_buf, struct aws_allocat AWS_ZERO_STRUCT(*out_buf); FILE *fp = aws_fopen(filename, "rb"); + if (fp == NULL) { + goto error; + } - if (fp) { - if (fseek(fp, 0L, SEEK_END)) { - int errno_value = errno; /* Always cache errno before potential side-effect */ - AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to seek file %s with errno %d", filename, errno_value); - fclose(fp); - return aws_translate_and_raise_io_error(errno_value); - } + int64_t len64 = 0; + if (aws_file_get_length(fp, &len64)) { + AWS_LOGF_ERROR( + AWS_LS_COMMON_IO, + "static: Failed to get file length. file:'%s' error:%s", + filename, + aws_error_name(aws_last_error())); + goto error; + } - size_t allocation_size = (size_t)ftell(fp) + 1; - /* Tell the user that we allocate here and if success they're responsible for the free. */ - if (aws_byte_buf_init(out_buf, alloc, allocation_size)) { - fclose(fp); - return AWS_OP_ERR; - } + if (len64 >= SIZE_MAX) { + aws_raise_error(AWS_ERROR_OVERFLOW_DETECTED); + AWS_LOGF_ERROR( + AWS_LS_COMMON_IO, + "static: File too large to read into memory. file:'%s' error:%s", + filename, + aws_error_name(aws_last_error())); + goto error; + } - /* Ensure compatibility with null-terminated APIs, but don't consider - * the null terminator part of the length of the payload */ - out_buf->len = out_buf->capacity - 1; - out_buf->buffer[out_buf->len] = 0; - - if (fseek(fp, 0L, SEEK_SET)) { - int errno_value = errno; /* Always cache errno before potential side-effect */ - AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to seek file %s with errno %d", filename, errno_value); - aws_byte_buf_clean_up(out_buf); - fclose(fp); - return aws_translate_and_raise_io_error(errno_value); - } + size_t allocation_size = (size_t)len64 + 1; + aws_byte_buf_init(out_buf, alloc, allocation_size); + + /* Ensure compatibility with null-terminated APIs, but don't consider + * the null terminator part of the length of the payload */ + out_buf->len = out_buf->capacity - 1; + out_buf->buffer[out_buf->len] = 0; + + size_t read = fread(out_buf->buffer, 1, out_buf->len, fp); + if (read < out_buf->len) { + int errno_value = ferror(fp) ? errno : 0; /* Always cache errno before potential side-effect */ + aws_translate_and_raise_io_error_or(errno_value, AWS_ERROR_FILE_READ_FAILURE); + AWS_LOGF_ERROR( + AWS_LS_COMMON_IO, + "static: Failed reading file:'%s' errno:%d aws-error:%s", + filename, + errno_value, + aws_error_name(aws_last_error())); + goto error; + } - size_t read = fread(out_buf->buffer, 1, out_buf->len, fp); - int errno_cpy = errno; /* Always cache errno before potential side-effect */ - fclose(fp); - if (read < out_buf->len) { - AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to read file %s with errno %d", filename, errno_cpy); - aws_secure_zero(out_buf->buffer, out_buf->len); - aws_byte_buf_clean_up(out_buf); - return aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE); - } + fclose(fp); + return AWS_OP_SUCCESS; - return AWS_OP_SUCCESS; +error: + if (fp) { + fclose(fp); } - + aws_byte_buf_clean_up_secure(out_buf); return AWS_OP_ERR; } diff --git a/source/log_writer.c b/source/log_writer.c index 5f5bc4f6f..9f5090627 100644 --- a/source/log_writer.c +++ b/source/log_writer.c @@ -27,8 +27,8 @@ static int s_aws_file_writer_write(struct aws_log_writer *writer, const struct a size_t length = output->len; if (fwrite(output->bytes, 1, length, impl->log_file) < length) { - int errno_value = errno; /* Always cache errno before potential side-effect */ - return aws_translate_and_raise_io_error(errno_value); + int errno_value = ferror(impl->log_file) ? errno : 0; /* Always cache errno before potential side-effect */ + return aws_translate_and_raise_io_error_or(errno_value, AWS_ERROR_FILE_WRITE_FAILURE); } return AWS_OP_SUCCESS; diff --git a/source/logging.c b/source/logging.c index d0e96c0b9..9033a208c 100644 --- a/source/logging.c +++ b/source/logging.c @@ -502,8 +502,8 @@ static int s_noalloc_stderr_logger_log( int write_result = AWS_OP_SUCCESS; if (fwrite(format_buffer, 1, format_data.amount_written, impl->file) < format_data.amount_written) { - int errno_value = errno; /* Always cache errno before potential side-effect */ - aws_translate_and_raise_io_error(errno_value); + int errno_value = ferror(impl->file) ? errno : 0; /* Always cache errno before potential side-effect */ + aws_translate_and_raise_io_error_or(errno_value, AWS_ERROR_FILE_WRITE_FAILURE); write_result = AWS_OP_ERR; } diff --git a/source/posix/file.c b/source/posix/file.c index f8dbbd7e1..868112955 100644 --- a/source/posix/file.c +++ b/source/posix/file.c @@ -18,7 +18,7 @@ FILE *aws_fopen_safe(const struct aws_string *file_path, const struct aws_string FILE *f = fopen(aws_string_c_str(file_path), aws_string_c_str(mode)); if (!f) { int errno_cpy = errno; /* Always cache errno before potential side-effect */ - aws_translate_and_raise_io_error(errno_cpy); + aws_translate_and_raise_io_error_or(errno_cpy, AWS_ERROR_FILE_OPEN_FAILURE); AWS_LOGF_ERROR( AWS_LS_COMMON_IO, "static: Failed to open file. path:'%s' mode:'%s' errno:%d aws-error:%d(%s)", @@ -285,7 +285,7 @@ int aws_fseek(FILE *file, int64_t offset, int whence) { int errno_value = errno; /* Always cache errno before potential side-effect */ if (result != 0) { - return aws_translate_and_raise_io_error(errno_value); + return aws_translate_and_raise_io_error_or(errno_value, AWS_ERROR_STREAM_UNSEEKABLE); } return AWS_OP_SUCCESS; diff --git a/source/windows/file.c b/source/windows/file.c index 47e874f07..7d4bb96d6 100644 --- a/source/windows/file.c +++ b/source/windows/file.c @@ -44,7 +44,7 @@ FILE *aws_fopen_safe(const struct aws_string *file_path, const struct aws_string aws_wstring_destroy(w_file_path); if (error) { - aws_translate_and_raise_io_error(error); + aws_translate_and_raise_io_error_or(error, AWS_ERROR_FILE_OPEN_FAILURE); AWS_LOGF_ERROR( AWS_LS_COMMON_IO, "static: Failed to open file. path:'%s' mode:'%s' errno:%d aws-error:%d(%s)", @@ -508,7 +508,7 @@ bool aws_path_exists(const struct aws_string *path) { int aws_fseek(FILE *file, int64_t offset, int whence) { if (_fseeki64(file, offset, whence)) { int errno_value = errno; /* Always cache errno before potential side-effect */ - return aws_translate_and_raise_io_error(errno_value); + return aws_translate_and_raise_io_error_or(errno_value, AWS_ERROR_STREAM_UNSEEKABLE); } return AWS_OP_SUCCESS;