Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

additional error handling when checking for file existence #946

Merged
merged 7 commits into from
Oct 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

* Fixed hard crash when attempting to read files within a read-protected directory ([#945](https://github.com/CARTAvis/carta-backend/issues/945)).
* Fixed region histograms for moment images ([#906](https://github.com/CARTAvis/carta-backend/issues/906)).
* Fixed bug of duplicate histogram calculation ([#905](https://github.com/CARTAvis/carta-backend/pull/905)).

Expand Down
3 changes: 2 additions & 1 deletion src/Frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1840,7 +1840,8 @@ bool Frame::ExportCASAImage(casacore::ImageInterface<casacore::Float>& image, fs
bool success(false);

// Remove the old image file if it has a same file name
if (fs::exists(output_filename)) {
std::error_code error_code;
if (fs::exists(output_filename, error_code)) {
fs::remove_all(output_filename);
}

Expand Down
4 changes: 3 additions & 1 deletion src/ImageData/CompressedFits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,9 @@ bool CompressedFits::DecompressedFileExists() {
SetDecompressFilename();

fs::path unzip_path(_unzip_filename);
if (fs::exists(unzip_path)) {
std::error_code error_code;

if (fs::exists(unzip_path, error_code)) {
// File already decompressed
spdlog::info("Using decompressed FITS file {}", _unzip_filename);
return true;
Expand Down
3 changes: 2 additions & 1 deletion src/ImageData/FitsLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ FitsLoader::FitsLoader(const std::string& filename, bool is_gz) : FileLoader(fil
FitsLoader::~FitsLoader() {
// Remove decompressed fits.gz file
auto unzip_path = fs::path(_unzip_file);
if (fs::exists(unzip_path)) {
std::error_code error_code;
if (fs::exists(unzip_path, error_code)) {
fs::remove(unzip_path);
}
}
Expand Down
23 changes: 16 additions & 7 deletions src/SessionManager/ProgramSettings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ ProgramSettings::ProgramSettings(int argc, char** argv) {
const fs::path system_settings_path = "/etc/carta/backend.json";

json settings;
if (fs::exists(system_settings_path) && !no_system_config) {
std::error_code error_code;

if (!no_system_config && fs::exists(system_settings_path, error_code)) {
settings = JSONSettingsFromFile(system_settings_path.string());
system_settings_json_exists = true;
debug_msgs.push_back(fmt::format("Reading system settings from {}.", system_settings_path.string()));
}

if (fs::exists(user_settings_path) && !no_user_config) {
if (!no_user_config && fs::exists(user_settings_path, error_code)) {
auto user_settings = JSONSettingsFromFile(user_settings_path.string());
user_settings_json_exists = true;
debug_msgs.push_back(fmt::format("Reading user settings from {}.", user_settings_path.string()));
Expand All @@ -61,6 +63,12 @@ ProgramSettings::ProgramSettings(int argc, char** argv) {
json ProgramSettings::JSONSettingsFromFile(const std::string& json_file_path) {
std::ifstream ifs(json_file_path, std::ifstream::in);
json j;

if (!ifs.good()) {
warning_msgs.push_back(fmt::format("Error reading config file {}.", json_file_path));
return j;
}

try {
j = json::parse(ifs, nullptr, true, true);
} catch (json::exception& err) {
Expand Down Expand Up @@ -269,8 +277,8 @@ end.
no_browser = result["no_browser"].as<bool>();
read_only_mode = result["read_only_mode"].as<bool>();

no_user_config = result.count("no_user_config") ? true : false;
no_system_config = result.count("no_system_config") ? true : false;
no_user_config = result.count("no_user_config") != 0;
no_system_config = result.count("no_system_config") != 0;
veggiesaurus marked this conversation as resolved.
Show resolved Hide resolved

applyOptionalArgument(top_level_folder, "root", result);
// Override deprecated "root" argument
Expand All @@ -295,8 +303,9 @@ end.

for (const auto& arg : positional_arguments) {
fs::path p(arg);
if (fs::exists(p)) {
if (fs::is_directory(p)) {
std::error_code error_code;
if (fs::exists(p, error_code)) {
if (fs::is_directory(p, error_code)) {
auto image_type = casacore::ImageOpener::imageType(p.string());
if (image_type == casacore::ImageOpener::AIPSPP || image_type == casacore::ImageOpener::MIRIAD ||
image_type == casacore::ImageOpener::IMAGECONCAT || image_type == casacore::ImageOpener::IMAGEEXPR ||
Expand All @@ -308,7 +317,7 @@ end.
file_paths.clear();
break;
}
} else if (!fs::is_regular_file(p)) {
} else if (!fs::is_regular_file(p, error_code)) {
// Ignore invalid files
file_paths.clear();
break;
Expand Down
4 changes: 3 additions & 1 deletion src/SessionManager/WebBrowser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ void WebBrowser::ParseCmd() {
std::istringstream isstream(_cmd);
std::copy(std::istream_iterator<std::string>(isstream), std::istream_iterator<std::string>(), std::back_inserter(_args));
fs::path path(_args[0]);
if (!fs::exists(path)) {
std::error_code error_code;

if (!fs::exists(path, error_code)) {
path = SearchPath(_args[0]);
}

Expand Down
31 changes: 20 additions & 11 deletions src/SimpleFrontendServer/SimpleFrontendServer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,19 @@ void SimpleFrontendServer::HandleStaticRequest(Res* res, Req* req) {
bool gzip_compressed = false;
auto gzip_path = path;
gzip_path += ".gz";
if (accepts_gzip && fs::exists(gzip_path) && fs::is_regular_file(gzip_path)) {
std::error_code error_code;
if (accepts_gzip && fs::exists(gzip_path, error_code) && fs::is_regular_file(gzip_path, error_code)) {
gzip_compressed = true;
path = gzip_path;
}

if (fs::exists(path) && fs::is_regular_file(path)) {
if (fs::exists(path, error_code) && fs::is_regular_file(path, error_code)) {
// Check file size
ifstream file(path.string(), ios::binary | ios::ate);
if (!file.good()) {
res->writeStatus(HTTP_404);
return;
}
streamsize size = file.tellg();
file.seekg(0, ios::beg);

Expand Down Expand Up @@ -113,13 +118,15 @@ void SimpleFrontendServer::HandleStaticRequest(Res* res, Req* req) {
}

bool SimpleFrontendServer::IsValidFrontendFolder(fs::path folder) {
std::error_code error_code;

// Check that the folder exists
if (!fs::exists(folder) || !fs::is_directory(folder)) {
if (!fs::exists(folder, error_code) || !fs::is_directory(folder, error_code)) {
return false;
}
// Check that index.html exists
folder /= "index.html";
if (!fs::exists(folder) || !fs::is_regular_file(folder)) {
if (!fs::exists(folder, error_code) || !fs::is_regular_file(folder, error_code)) {
return false;
}
// Check that index.html can be read
Expand All @@ -139,15 +146,15 @@ void SimpleFrontendServer::AddNoCacheHeaders(Res* res) {

json SimpleFrontendServer::GetExistingPreferences() {
auto preferences_path = _config_folder / "preferences.json";
if (!fs::exists(preferences_path)) {
return {{"version", 1}};
}

try {
if (!fs::exists(preferences_path)) {
return {{"version", 1}};
}
ifstream file(preferences_path.string());
string json_string((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>());
return json::parse(json_string);
veggiesaurus marked this conversation as resolved.
Show resolved Hide resolved
} catch (exception) {
} catch (json::exception e) {
spdlog::warn(e.what());
return {};
}
}
Expand Down Expand Up @@ -359,13 +366,15 @@ void SimpleFrontendServer::HandleClearObject(const std::string& object_type, Res
nlohmann::json SimpleFrontendServer::GetExistingObjects(const std::string& object_type) {
auto object_folder = _config_folder / (object_type + "s");
json objects = json::object();
if (fs::exists(object_folder)) {
std::error_code error_code;

if (fs::exists(object_folder, error_code)) {
for (auto& p : fs::directory_iterator(object_folder)) {
try {
string filename = p.path().filename().string();
jolopezl marked this conversation as resolved.
Show resolved Hide resolved
regex object_regex(R"(^(.+)\.json$)");
smatch sm;
if (fs::is_regular_file(p) && regex_search(filename, sm, object_regex) && sm.size() == 2) {
if (fs::is_regular_file(p, error_code) && regex_search(filename, sm, object_regex) && sm.size() == 2) {
string object_name = sm[1];
ifstream file(p.path().string());
string json_string((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>());
Expand Down
11 changes: 8 additions & 3 deletions src/Table/Table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ using namespace std;

Table::Table(const string& filename, bool header_only) : _valid(false), _filename(filename), _num_rows(0), _available_rows(0) {
fs::path file_path(filename);

if (!fs::exists(file_path)) {
_parse_error_message = "File does not exist!";
std::error_code error_code;
if (!fs::exists(file_path, error_code)) {
_parse_error_message = "File does not exist or cannot be read!";
return;
}

Expand All @@ -43,6 +43,11 @@ Table::Table(const string& filename, bool header_only) : _valid(false), _filenam
string Table::GetHeader(const string& filename) {
ifstream in(filename);
string header_string;

if (!in.good()) {
return header_string;
}

// Measure entire file size to ensure we don't read past EOF
in.seekg(0, ios_base::end);
size_t header_size = min(size_t(in.tellg()), size_t(MAX_HEADER_SIZE));
Expand Down
20 changes: 15 additions & 5 deletions src/Table/TableController.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ void TableController::OnOpenFileRequest(const CARTA::OpenCatalogFile& open_file_

open_file_response.set_file_id(file_id);
auto file_path = GetPath(open_file_request.directory(), open_file_request.name());
if (!fs::exists(file_path) || !fs::is_regular_file(file_path)) {
std::error_code error_code;

if (!fs::exists(file_path, error_code) || !fs::is_regular_file(file_path, error_code)) {
open_file_response.set_message(fmt::format("Cannot find path {}", file_path.string()));
open_file_response.set_success(false);
return;
Expand Down Expand Up @@ -186,8 +188,9 @@ void TableController::OnFileListRequest(
const CARTA::CatalogListRequest& file_list_request, CARTA::CatalogListResponse& file_list_response) {
fs::path root_path(_top_level_folder);
fs::path file_path = GetPath(file_list_request.directory());
std::error_code error_code;

if (!fs::exists(file_path) || !fs::is_directory(file_path)) {
if (!fs::exists(file_path, error_code) || !fs::is_directory(file_path, error_code)) {
file_list_response.set_success(false);
file_list_response.set_message("Incorrect file path");
return;
Expand Down Expand Up @@ -221,7 +224,13 @@ void TableController::OnFileListRequest(
break;
}

if (fs::is_directory(entry)) {
// Skip files that can't be read
std::error_code error_code;
if (!fs::exists(entry, error_code)) {
continue;
}

if (fs::is_directory(entry, error_code)) {
try {
// Try to construct a directory iterator. If it fails, the directory is inaccessible
auto test_directory_iterator = fs::directory_iterator(entry);
Expand All @@ -238,7 +247,7 @@ void TableController::OnFileListRequest(
// Skip inaccessible folders
continue;
}
} else if (fs::is_regular_file(entry) && fs::exists(entry)) {
} else if (fs::is_regular_file(entry, error_code)) {
uint32_t file_magic_number = GetMagicNumber(entry.path().string());
CARTA::CatalogFileType file_type;
if (file_magic_number == XML_MAGIC_NUMBER) {
Expand Down Expand Up @@ -282,8 +291,9 @@ void TableController::OnFileListRequest(
void TableController::OnFileInfoRequest(
const CARTA::CatalogFileInfoRequest& file_info_request, CARTA::CatalogFileInfoResponse& file_info_response) {
fs::path file_path = GetPath(file_info_request.directory(), file_info_request.name());
std::error_code error_code;

if (!fs::exists(file_path) || !fs::is_regular_file(file_path)) {
if (!fs::exists(file_path, error_code) || !fs::is_regular_file(file_path, error_code)) {
file_info_response.set_success(false);
file_info_response.set_message("Incorrect file path");
return;
Expand Down
11 changes: 7 additions & 4 deletions src/Util/App.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,17 @@ std::string GetReleaseInformation() {
// Unix solution just attempts to read from /etc/os-release. This works with Ubuntu, RedHat, CentOS, Arch, Debian and Fedora,
// and should work on any system that has systemd installed
fs::path path = "/etc/os-release";
std::error_code error_code;

if (fs::exists(path) && fs::is_regular_file(path)) {
if (fs::exists(path, error_code) && fs::is_regular_file(path, error_code)) {
try {
// read the entire release file to string
std::ifstream input_file(path);
std::stringstream buffer;
buffer << input_file.rdbuf();
return buffer.str();
if (input_file.good()) {
std::stringstream buffer;
buffer << input_file.rdbuf();
return buffer.str();
}
} catch (std::ifstream::failure e) {
spdlog::warn("Problem reading platform information");
}
Expand Down
19 changes: 12 additions & 7 deletions src/Util/File.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ uint32_t GetMagicNumber(const std::string& filename) {
uint32_t magic_number = 0;

std::ifstream input_file(filename);
if (input_file) {
if (input_file && input_file.good()) {
input_file.read((char*)&magic_number, sizeof(magic_number));
input_file.close();
}
Expand Down Expand Up @@ -42,7 +42,7 @@ int GetNumItems(const std::string& path) {
counter++;
}
return counter;
} catch (std::exception) {
} catch (fs::filesystem_error) {
return -1;
}
}
Expand All @@ -53,12 +53,17 @@ fs::path SearchPath(std::string filename) {
std::string path(std::getenv("PATH"));
std::vector<std::string> path_strings;
SplitString(path, ':', path_strings);
for (auto& p : path_strings) {
fs::path base_path(p);
base_path /= filename;
if (fs::exists(base_path)) {
return base_path;

try {
for (auto& p : path_strings) {
fs::path base_path(p);
base_path /= filename;
if (fs::exists(base_path)) {
return base_path;
}
}
} catch (fs::filesystem_error) {
return fs::path();
}
return fs::path();
}