Check illegal characters from the file and parent folder name

PiperOrigin-RevId: 634941778
This commit is contained in:
Eiden Kim
2024-05-17 18:46:26 -07:00
committed by Copybara-Service
parent 4ff7c1511c
commit b224f8660c
5 changed files with 72 additions and 144 deletions
@@ -406,13 +406,6 @@ Exception EnsureValidOfflineFrame(
.payload_transfer()
.payload_header()
.has_file_name()) {
if (offline_frame.v1()
.payload_transfer()
.payload_header()
.file_name()
.empty()) {
return {Exception::kIllegalCharacters};
}
if (CheckForIllegalCharacters(offline_frame.v1()
.payload_transfer()
.payload_header()
@@ -421,8 +414,6 @@ Exception EnsureValidOfflineFrame(
kIllegalFileNamePatternsSize)) {
return {Exception::kIllegalCharacters};
}
} else { // Filename is empty.
return {Exception::kIllegalCharacters};
}
if (offline_frame.v1()
.payload_transfer()
@@ -28,8 +28,7 @@ namespace parser {
constexpr absl::string_view kIllegalFileNamePatterns[] = {":", "/", "\\"};
constexpr absl::string_view kIllegalParentFolderPatterns[] = {":", "../",
"..\\"};
constexpr absl::string_view kIllegalParentFolderPatterns[] = {":", ".."};
const size_t kIllegalFileNamePatternsSize =
sizeof(kIllegalFileNamePatterns) / sizeof(*kIllegalFileNamePatterns);
@@ -22,7 +22,6 @@
#include "connections/implementation/offline_frames.h"
#include "connections/implementation/proto/offline_wire_formats.pb.h"
#include "internal/platform/byte_array.h"
#include "internal/platform/logging.h"
namespace nearby {
namespace connections {
@@ -247,7 +246,7 @@ TEST(OfflineFramesValidatorTest,
// Sending files larger than 2gb was previously broken (see cl/372382338).
// This tests a file larger than int max.
header.set_total_size(3e10);
header.set_file_name("file_name.jpg");
header.set_file_name(std::string());
header.set_parent_folder(std::string());
chunk.set_body("payload data");
chunk.set_offset(150);
@@ -288,6 +287,31 @@ TEST(OfflineFramesValidatorTest, ValidatesAsOkTypeFileWithLegalFilePath) {
ASSERT_TRUE(ret_value.Ok());
}
TEST(OfflineFramesValidatorTest, ValidatesAsFailedTypeFileWithIllegalFilePath) {
PayloadTransferFrame::PayloadHeader header;
PayloadTransferFrame::PayloadChunk chunk;
header.set_id(12345);
header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
// Sending files larger than 2gb was previously broken (see cl/372382338).
// This tests a file larger than int max.
header.set_total_size(3e10);
header.set_file_name(
std::string("earth_85MB_test (1): (3) (4) (8) (1) (2) (2) (1).jpg"));
header.set_parent_folder(std::string());
chunk.set_body("payload data");
chunk.set_offset(150);
chunk.set_flags(1);
OfflineFrame offline_frame;
ByteArray bytes = ForDataPayloadTransfer(header, chunk);
offline_frame.ParseFromString(std::string(bytes));
auto ret_value = EnsureValidOfflineFrame(offline_frame);
ASSERT_TRUE(ret_value.value == Exception::kIllegalCharacters);
}
TEST(OfflineFramesValidatorTest, ValidatesAsOkTypeFileWithLegalParentFolder) {
PayloadTransferFrame::PayloadHeader header;
PayloadTransferFrame::PayloadChunk chunk;
@@ -296,7 +320,7 @@ TEST(OfflineFramesValidatorTest, ValidatesAsOkTypeFileWithLegalParentFolder) {
// Sending files larger than 2gb was previously broken (see cl/372382338).
// This tests a file larger than int max.
header.set_total_size(3e10);
header.set_file_name("test_file.txt");
header.set_file_name("");
header.set_parent_folder(std::string(
std::string("earth_85MB_test (1) (3) (4) (8) (1) (2) (2) (1).jpg")));
chunk.set_body("payload data");
@@ -313,6 +337,31 @@ TEST(OfflineFramesValidatorTest, ValidatesAsOkTypeFileWithLegalParentFolder) {
ASSERT_TRUE(ret_value.Ok());
}
TEST(OfflineFramesValidatorTest,
ValidatesAsFailedTypeFileWithIllegalParentFolder) {
PayloadTransferFrame::PayloadHeader header;
PayloadTransferFrame::PayloadChunk chunk;
header.set_id(12345);
header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
// Sending files larger than 2gb was previously broken (see cl/372382338).
// This tests a file larger than int max.
header.set_total_size(3e10);
header.set_file_name("");
header.set_parent_folder(std::string(
std::string("earth_85MB_test (1): (3) (4) (8) (1) (2) (2) (1).jpg")));
chunk.set_body("payload data");
chunk.set_offset(150);
chunk.set_flags(1);
OfflineFrame offline_frame;
ByteArray bytes = ForDataPayloadTransfer(header, chunk);
offline_frame.ParseFromString(std::string(bytes));
auto ret_value = EnsureValidOfflineFrame(offline_frame);
ASSERT_TRUE(ret_value.value == Exception::kIllegalCharacters);
}
TEST(OfflineFramesValidatorTest, ValidatesAsFailWithNullPayloadTransferFrame) {
PayloadTransferFrame::PayloadHeader header;
PayloadTransferFrame::PayloadChunk chunk;
@@ -667,66 +716,6 @@ TEST(OfflineFramesValidatorTest,
ASSERT_FALSE(ret_value.Ok());
}
struct FileNameParentFolderTestData {
std::string test_title;
std::string file_name;
std::string parent_folder;
std::string expected;
};
using FileNameParentFolderValidationTest =
testing::TestWithParam<FileNameParentFolderTestData>;
TEST_P(FileNameParentFolderValidationTest, CheckIllegalFileNameWithParam) {
NEARBY_LOGS(INFO) << "Running FileNameParentFolderValidationTest_"
<< GetParam().test_title;
PayloadTransferFrame::PayloadHeader header;
PayloadTransferFrame::PayloadChunk chunk;
header.set_id(12345);
header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
// Sending files larger than 2gb was previously broken (see cl/372382338).
// This tests a file larger than int max.
header.set_total_size(3e10);
header.set_file_name(GetParam().file_name);
header.set_parent_folder(GetParam().parent_folder);
chunk.set_body("payload data");
chunk.set_offset(150);
chunk.set_flags(1);
OfflineFrame offline_frame;
ByteArray bytes = ForDataPayloadTransfer(header, chunk);
offline_frame.ParseFromString(std::string(bytes));
auto ret_value = EnsureValidOfflineFrame(offline_frame);
if (GetParam().expected == "exception") {
ASSERT_TRUE(ret_value.value == Exception::kIllegalCharacters);
} else {
ASSERT_TRUE(ret_value.Ok());
}
}
INSTANTIATE_TEST_SUITE_P(
FileNameParentFolderTestInitiation, // This name is only used for
// instantiation
FileNameParentFolderValidationTest, // This is the name of your
// parameterized test
testing::ValuesIn<FileNameParentFolderTestData>({
{"CheckLegalFileName", "file_name.jpg", "", "no exception"},
{"CheckEmptyFileName", "", "", "exception"},
{"CheckIllegalFileNameColon", "file_name:.jpg", "", "exception"},
{"CheckIllegalFileNameSlash", "file_name/.jpg", "", "exception"},
{"CheckIllegalFileNameBackSlash", "file_name\\.jpg", "", "exception"},
{"CheckLegalParentFolder", "file_name.txt", "parent_/folder",
"no exception"},
{"CheckIllegalParentFolderColon", "file_name.jpg", "parent:/folder",
"exception"},
{"CheckIllegalParentFolderDoubleDotSlash", "file_name.txt",
"parent../folder", "exception"},
{"CheckIllegalParentFolderDoubleDotBackSlash", "file_name.txt",
"parent..\\folder", "exception"},
}));
} // namespace
} // namespace parser
} // namespace connections
@@ -37,19 +37,16 @@
namespace nearby {
namespace windows {
const wchar_t* kUpOneLevel = L"../";
constexpr wchar_t kEndDot = L'.';
const wchar_t* kUpOneLevel = L"/..";
constexpr wchar_t kPathDelimiter = L'/';
constexpr wchar_t kReplacementChar = L'_';
constexpr wchar_t kForwardSlash = L'/';
constexpr wchar_t kBackSlash = L'\\';
const wchar_t kIllegalFileCharacters[] = {L'*', L'?', L'\"', L'<', L'>', L'|'};
const wchar_t* const kForbiddenPathNames[] = {
L"CON", L"PRN", L"AUX", L"NUL", L"COM1", L"COM2", L"COM3",
L"COM4", L"COM5", L"COM6", L"COM7", L"COM8", L"COM9", L"COM¹",
L"COM²", L"COM³", L"LPT1", L"LPT2", L"LPT3", L"LPT4", L"LPT5",
L"LPT6", L"LPT7", L"LPT8", L"LPT9", L"LPT¹", L"LPT²", L"LPT³"};
wchar_t const* kForbiddenPathNames[] = {
L"CON", L"PRN", L"AUX", L"NUL", L"COM1", L"COM2", L"COM3", L"COM4",
L"COM5", L"COM6", L"COM7", L"COM8", L"COM9", L"LPT1", L"LPT2", L"LPT3",
L"LPT4", L"LPT5", L"LPT6", L"LPT7", L"LPT8", L"LPT9"};
std::wstring FilePath::GetCustomSavePath(std::wstring parent_folder,
std::wstring file_name) {
@@ -131,6 +128,7 @@ std::wstring FilePath::CreateOutputFileWithRename(std::wstring path) {
std::replace(sanitized_path.begin(), sanitized_path.end(), kBackSlash,
kForwardSlash);
// Remove any /..'s
SanitizePath(sanitized_path);
auto last_delimiter = sanitized_path.find_last_of(kPathDelimiter);
@@ -236,21 +234,26 @@ void FilePath::SanitizePath(std::wstring& path) {
// If found then erase it from string
path.erase(pos, wcslen(kUpOneLevel));
}
while (path[path.size() - 1] == kEndDot) {
// If found then erase it from string
path.erase(path.size() - 1, 1);
}
path = MutateForbiddenPathElements(path);
ReplaceInvalidCharacters(path);
}
char kIllegalFileCharacters[] = {'?', '*', '\'', '<', '>', '|', ':'};
void FilePath::ReplaceInvalidCharacters(std::wstring& path) {
auto it = path.begin();
it += 2; // Skip the 'C:' or any other drive specifier
for (; it != path.end(); it++) {
// If 0 < character < 32, it's illegal, replace it
if (*it > 0 && *it < 32) {
NEARBY_LOGS(INFO) << "In path " << wstring_to_string(path)
<< " replaced \'" << std::string(1, *it) << "\' with \'"
<< std::string(1, kReplacementChar);
*it = kReplacementChar;
}
for (auto illegal_character : kIllegalFileCharacters) {
if (*it == illegal_character) {
NEARBY_LOGS(INFO) << "In path " << wstring_to_string(path)
@@ -259,19 +262,6 @@ void FilePath::ReplaceInvalidCharacters(std::wstring& path) {
*it = kReplacementChar;
}
}
if (*it > 0 &&
*it < 32) { // If 0 < character < 32, it's illegal, replace it
NEARBY_LOGS(INFO) << "In path " << wstring_to_string(path)
<< " replaced \'" << std::string(1, *it) << "\' with \'"
<< std::string(1, kReplacementChar);
*it = kReplacementChar;
}
if (*it == 0) { // character is null
NEARBY_LOGS(INFO) << "In path " << wstring_to_string(path)
<< " replaced \'NULL\' with \'"
<< std::string(1, kReplacementChar);
*it = kReplacementChar;
}
}
}
@@ -31,16 +31,14 @@ namespace windows {
namespace {
const wchar_t* kIllegalPathNames[] = {
L"CON", L"PRN", L"AUX", L"NUL", L"COM1", L"COM2", L"COM3",
L"COM4", L"COM5", L"COM6", L"COM7", L"COM8", L"COM9", L"COM¹",
L"COM²", L"COM³", L"LPT1", L"LPT2", L"LPT3", L"LPT4", L"LPT5",
L"LPT6", L"LPT7", L"LPT8", L"LPT9", L"LPT¹", L"LPT²", L"LPT³"};
L"CON", L"PRN", L"AUX", L"NUL", L"COM1", L"COM2", L"COM3", L"COM4",
L"COM5", L"COM6", L"COM7", L"COM8", L"COM9", L"LPT1", L"LPT2", L"LPT3",
L"LPT4", L"LPT5", L"LPT6", L"LPT7", L"LPT8", L"LPT9"};
const wchar_t* kFileName(L"increment_file_test.txt");
const wchar_t* kFirstIterationFileName(L"/increment_file_test (1).txt");
const wchar_t* kSecondIterationFileName(L"/increment_file_test (2).txt");
const wchar_t* kThirdIterationFileName(L"/increment_file_test (3).txt");
const wchar_t* kFileNameWithNullReplaced(L"/increment_file_test.txt_.txt");
const wchar_t* kNoDotsFileName(L"incrementfiletesttxt");
const wchar_t* kOneIterationNoDotsFileName(L"/incrementfiletesttxt (1)");
const wchar_t* kMultipleDotsFileName(L"increment.file.test.txt");
@@ -593,7 +591,7 @@ TEST_F(FilePathTests, GetDownloadPath_IllegalFileNameCharacterColon\
ReturnsFileNameWithUnderbarSubstituted) {
// char illegal_character_sequence[]{ 0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x2f,
// 0x77, 0x6f, 0x72, 0x6c, 0x64, 0x21, 0 };
auto illegal_character_sequence(L"Test\"Test");
auto illegal_character_sequence(L"Test:Test");
std::wstring parent_folder(L"");
@@ -656,45 +654,6 @@ FileWithIncrementedName) {
ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
}
TEST_F(FilePathTests, GetDownloadPath_FileExistsReturns\
FileWithIncrementedNameWithNull) {
std::wstring file_name(kFileName);
int size = file_name.size();
file_name.append(L"1.txt");
file_name[size] = L'\x00';
std::wstring renamed_file_name(kFileNameWithNullReplaced);
std::wstring parent_folder(L"");
std::wstring output_file_path(default_download_path_);
output_file_path.append(L"/");
output_file_path.append(file_name);
std::wstring expected(default_download_path_);
expected += renamed_file_name;
std::wifstream input_file;
std::wofstream output_file;
output_file.open(output_file_path,
std::ofstream::binary | std::ofstream::out);
ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
output_file.close();
auto actual(FilePath::GetDownloadPath(parent_folder, file_name));
EXPECT_EQ(actual, expected);
// Remove the file and check that it is removed
// File 1
_wremove(output_file_path.c_str());
input_file.open(output_file_path, std::ifstream::binary | std::ifstream::in);
ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
}
TEST_F(FilePathTests, GetDownloadPath_MultipleFilesExist\
ReturnsNextIncrementedFileName) {
std::ofstream output_file;