Skip to content

basicio: use fs::path #3264

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

basicio: use fs::path #3264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5596e36
basicio: use fs::path
neheb Apr 26, 2025
50b7aa1
fixup wstring functions to use fs::path
neheb Apr 26, 2025
9680bb4
unicode test
neheb Apr 27, 2025
c2aaba9
maybe?
neheb Apr 28, 2025
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
3 changes: 3 additions & 0 deletions include/exiv2/basicio.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,9 @@ class EXIV2API FileIo : public BasicIo {
Nonzero if failure.
*/
int open(const std::string& mode);
#ifdef _WIN32
int open(const std::wstring& mode);
#endif
/*!
@brief Open the file using the default access mode of "rb".
This method can also be used to "reopen" a file which will flush
Expand Down
10 changes: 8 additions & 2 deletions include/exiv2/image.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ class EXIV2API ImageFactory {
*/
static BasicIo::UniquePtr createIo(const std::string& path, bool useCurl = true);
#ifdef _WIN32
static BasicIo::UniquePtr createIo(const std::wstring& path);
static BasicIo::UniquePtr createIo(const std::wstring& path, bool useCurl = true);
#endif
/*!
@brief Create an Image subclass of the appropriate type by reading
Expand All @@ -540,7 +540,7 @@ class EXIV2API ImageFactory {
*/
static Image::UniquePtr open(const std::string& path, bool useCurl = true);
#ifdef _WIN32
static Image::UniquePtr open(const std::wstring& path);
static Image::UniquePtr open(const std::wstring& path, bool useCurl = true);
#endif
/*!
@brief Create an Image subclass of the appropriate type by reading
Expand Down Expand Up @@ -582,6 +582,9 @@ class EXIV2API ImageFactory {
@throw Error If the image type is not supported.
*/
static Image::UniquePtr create(ImageType type, const std::string& path);
#ifdef _WIN32
static Image::UniquePtr create(ImageType type, const std::wstring& path);
#endif
/*!
@brief Create an Image subclass of the requested type by creating a
new image in memory.
Expand Down Expand Up @@ -615,6 +618,9 @@ class EXIV2API ImageFactory {
@return %Image type or Image::none if the type is not recognized.
*/
static ImageType getType(const std::string& path);
#ifdef _WIN32
static ImageType getType(const std::wstring& path);
#endif
/*!
@brief Returns the image type of the provided data buffer.
@param data Pointer to a data buffer containing an image. The contents
Expand Down
95 changes: 46 additions & 49 deletions src/basicio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,17 @@ void BasicIo::seekOrThrow(int64_t offset, Position pos, ErrorCode err) {
class FileIo::Impl {
public:
//! Constructor
explicit Impl(std::string path);
#ifdef _WIN32
explicit Impl(std::wstring path);
#endif
explicit Impl(fs::path path);
~Impl() = default;
// Enumerations
//! Mode of operation
enum OpMode { opRead, opWrite, opSeek };
// DATA
std::string path_; //!< (Standard) path
#ifdef _WIN32
std::wstring wpath_; //!< UCS2 path
#endif
std::string openMode_; //!< File open mode
FILE* fp_{}; //!< File stream pointer
OpMode opMode_{opSeek}; //!< File open mode
fs::path path_; //!< (Standard) path
std::string openMode_; //!< File open mode
std::wstring wOpenMode_; //!< File open mode (wide)
FILE* fp_{}; //!< File stream pointer
OpMode opMode_{opSeek}; //!< File open mode

#if defined _WIN32
HANDLE hFile_{}; //!< Duplicated fd
Expand Down Expand Up @@ -113,21 +108,8 @@ class FileIo::Impl {
Impl& operator=(const Impl&) = delete; //!< Assignment
};

FileIo::Impl::Impl(std::string path) : path_(std::move(path)) {
#ifdef _WIN32
wchar_t t[512];
const auto nw = MultiByteToWideChar(CP_UTF8, 0, path_.data(), static_cast<int>(path_.size()), t, 512);
wpath_.assign(t, nw);
#endif
FileIo::Impl::Impl(fs::path path) : path_(std::move(path)) {
}
#ifdef _WIN32
FileIo::Impl::Impl(std::wstring path) : wpath_(std::move(path)) {
char t[1024];
const auto nc =
WideCharToMultiByte(CP_UTF8, 0, wpath_.data(), static_cast<int>(wpath_.size()), t, 1024, nullptr, nullptr);
path_.assign(t, nc);
}
#endif

int FileIo::Impl::switchMode(OpMode opMode) {
if (opMode_ == opMode)
Expand All @@ -137,6 +119,19 @@ int FileIo::Impl::switchMode(OpMode opMode) {

bool reopen = true;
switch (opMode) {
#ifdef _WIN32
case opRead:
// Flush if current mode allows reading, else reopen (in mode "r+b"
// as in this case we know that we can write to the file)
if (wOpenMode_.front() == L'r' || wOpenMode_.at(1) == L'+')
reopen = false;
break;
case opWrite:
// Flush if current mode allows writing, else reopen
if (wOpenMode_.front() != L'r' || wOpenMode_.at(1) == L'+')
reopen = false;
break;
#else
case opRead:
// Flush if current mode allows reading, else reopen (in mode "r+b"
// as in this case we know that we can write to the file)
Expand All @@ -148,6 +143,7 @@ int FileIo::Impl::switchMode(OpMode opMode) {
if (openMode_.front() != 'r' || openMode_.at(1) == '+')
reopen = false;
break;
#endif
case opSeek:
reopen = false;
break;
Expand Down Expand Up @@ -177,7 +173,7 @@ int FileIo::Impl::switchMode(OpMode opMode) {
openMode_ = "r+b";
opMode_ = opSeek;
#ifdef _WIN32
if (_wfopen_s(&fp_, wpath_.c_str(), L"r+b"))
if (_wfopen_s(&fp_, path_.c_str(), L"r+b"))
return 1;
return _fseeki64(fp_, offset, SEEK_SET);
#else
Expand All @@ -189,11 +185,7 @@ int FileIo::Impl::switchMode(OpMode opMode) {
} // FileIo::Impl::switchMode

int FileIo::Impl::stat(StructStat& buf) const {
#ifdef _WIN32
const auto& file = wpath_;
#else
const auto& file = path_;
#endif
try {
buf.st_size = fs::file_size(file);
buf.st_mode = fs::status(file).permissions();
Expand Down Expand Up @@ -320,21 +312,12 @@ byte* FileIo::mmap(bool isWriteable) {
void FileIo::setPath(const std::string& path) {
close();
p_->path_ = path;
#ifdef _WIN32
wchar_t t[512];
const auto nw = MultiByteToWideChar(CP_UTF8, 0, p_->path_.data(), static_cast<int>(p_->path_.size()), t, 512);
p_->wpath_.assign(t, nw);
#endif
}

#ifdef _WIN32
void FileIo::setPath(const std::wstring& path) {
close();
p_->wpath_ = path;
char t[1024];
const auto nc = WideCharToMultiByte(CP_UTF8, 0, p_->wpath_.data(), static_cast<int>(p_->wpath_.size()), t, 1024,
nullptr, nullptr);
p_->path_.assign(t, nc);
p_->path_ = path;
}
#endif

Expand Down Expand Up @@ -497,7 +480,11 @@ size_t FileIo::tell() const {

size_t FileIo::size() const {
// Flush and commit only if the file is open for writing
#ifdef _WIN32
if (p_->fp_ && (p_->wOpenMode_.front() != L'r' || p_->wOpenMode_.at(1) == L'+')) {
#else
if (p_->fp_ && (p_->openMode_.front() != 'r' || p_->openMode_.at(1) == '+')) {
#endif
std::fflush(p_->fp_);
#ifdef _MSC_VER
// This is required on msvcrt before stat after writing to a file
Expand All @@ -513,26 +500,34 @@ size_t FileIo::size() const {

int FileIo::open() {
// Default open is in read-only binary mode
#ifdef _WIN32
return open(L"rb");
#else
return open("rb");
#endif
}

int FileIo::open(const std::string& mode) {
close();
p_->openMode_ = mode;
p_->opMode_ = Impl::opSeek;
#ifdef _WIN32
wchar_t wmode[10];
MultiByteToWideChar(CP_UTF8, 0, mode.c_str(), -1, wmode, 10);
if (_wfopen_s(&p_->fp_, p_->wpath_.c_str(), wmode))
return 1;
#else
p_->fp_ = ::fopen(path().c_str(), mode.c_str());
if (!p_->fp_)
return 1;
#endif
return 0;
}

#ifdef _WIN32
int FileIo::open(const std::wstring& mode) {
close();
p_->wOpenMode_ = mode;
p_->opMode_ = Impl::opSeek;
if (_wfopen_s(&p_->fp_, p_->path_.c_str(), mode.c_str()))
return 1;
return 0;
}
#endif

bool FileIo::isopen() const {
return p_->fp_ != nullptr;
}
Expand Down Expand Up @@ -583,7 +578,9 @@ bool FileIo::eof() const {
}

const std::string& FileIo::path() const noexcept {
return p_->path_;
static thread_local std::string p;
p = p_->path_.string();
return p;
}

void FileIo::populateFakeData() {
Expand Down Expand Up @@ -870,7 +867,7 @@ bool MemIo::eof() const {
}

const std::string& MemIo::path() const noexcept {
static std::string _path{"MemIo"};
static const std::string _path{"MemIo"};
return _path;
}

Expand Down
17 changes: 14 additions & 3 deletions src/image.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,17 @@ ImageType ImageFactory::getType([[maybe_unused]] const std::string& path) {
#endif
}

#ifdef _WIN32
ImageType ImageFactory::getType([[maybe_unused]] const std::wstring& path) {
#ifdef EXV_ENABLE_FILESYSTEM
FileIo fileIo(path);
return getType(fileIo);
#else
return ImageType::none;
#endif
}
#endif

ImageType ImageFactory::getType(const byte* data, size_t size) {
MemIo memIo(data, size);
return getType(memIo);
Expand Down Expand Up @@ -806,7 +817,7 @@ BasicIo::UniquePtr ImageFactory::createIo(const std::string& path, [[maybe_unuse
} // ImageFactory::createIo

#ifdef _WIN32
BasicIo::UniquePtr ImageFactory::createIo(const std::wstring& path) {
BasicIo::UniquePtr ImageFactory::createIo(const std::wstring& path, bool) {
#ifdef EXV_ENABLE_FILESYSTEM
return std::make_unique<FileIo>(path);
#else
Expand All @@ -823,8 +834,8 @@ Image::UniquePtr ImageFactory::open(const std::string& path, bool useCurl) {
}

#ifdef _WIN32
Image::UniquePtr ImageFactory::open(const std::wstring& path) {
auto image = open(ImageFactory::createIo(path)); // may throw
Image::UniquePtr ImageFactory::open(const std::wstring& path, bool useCurl) {
auto image = open(ImageFactory::createIo(path, useCurl)); // may throw
if (!image) {
char t[1024];
WideCharToMultiByte(CP_UTF8, 0, path.c_str(), -1, t, 1024, nullptr, nullptr);
Expand Down
Binary file added test/data/Реган.jp2
View file
Open in desktop
Binary file not shown.
1 change: 1 addition & 0 deletions tests/regression_tests/test_regression_allfiles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ def get_valid_files(data_dir):
"imagemagick.pgf",
"iptc-psAPP13s-wIPTC-psAPP13s-noIPTC.jpg",
"Reagan.jp2",
"Реган.jp2",
"issue_ghsa_8949_hhfh_j7rj_poc.exv",
"exiv2-bug495.jpg",
"issue_1920_poc.tiff",
Expand Down
24 changes: 14 additions & 10 deletions unitTests/test_ImageFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,45 +100,49 @@ TEST(TheImageFactory, cannotCreateInstancesForSomeTypesInFiles) {
TEST(TheImageFactory, loadInstancesDifferentImageTypes) {
fs::path testData(TESTDATA_PATH);

std::string imagePath = (testData / "DSC_3079.jpg").string();
fs::path imagePath = testData / "DSC_3079.jpg";
EXPECT_EQ(ImageType::jpeg, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

imagePath = (testData / "exiv2-bug1108.exv").string();
imagePath = testData / "exiv2-bug1108.exv";
EXPECT_EQ(ImageType::exv, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

imagePath = (testData / "exiv2-canon-powershot-s40.crw").string();
imagePath = testData / "exiv2-canon-powershot-s40.crw";
EXPECT_EQ(ImageType::crw, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

imagePath = (testData / "exiv2-bug1044.tif").string();
imagePath = testData / "exiv2-bug1044.tif";
EXPECT_EQ(ImageType::tiff, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

#ifdef EXV_HAVE_LIBZ
imagePath = (testData / "exiv2-bug1074.png").string();
imagePath = testData / "exiv2-bug1074.png";
EXPECT_EQ(ImageType::png, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));
#endif

imagePath = (testData / "BlueSquare.xmp").string();
imagePath = testData / "BlueSquare.xmp";
EXPECT_EQ(ImageType::xmp, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

imagePath = (testData / "exiv2-photoshop.psd").string();
imagePath = testData / "exiv2-photoshop.psd";
EXPECT_EQ(ImageType::psd, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

imagePath = (testData / "cve_2017_1000126_stack-oob-read.webp").string();
imagePath = testData / "cve_2017_1000126_stack-oob-read.webp";
EXPECT_EQ(ImageType::webp, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

imagePath = (testData / "imagemagick.pgf").string();
imagePath = testData / "imagemagick.pgf";
EXPECT_EQ(ImageType::pgf, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

imagePath = (testData / "Reagan.jp2").string();
imagePath = testData / "Reagan.jp2";
EXPECT_EQ(ImageType::jp2, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));

imagePath = testData / "Реган.jp2";
EXPECT_EQ(ImageType::jp2, ImageFactory::getType(imagePath));
EXPECT_NO_THROW(ImageFactory::open(imagePath, false));
}
Expand Down
Loading