Skip to content

Commit

Permalink
fix: use vector::at() rather than operator[] (#1735)
Browse files Browse the repository at this point in the history
* Regression test for #1706

* Use vector::at() rather than operator[].

* Print to stderr when exception is caught and EXIV2_DEBUG_MESSAGES is enabled.

* Check that it prints "Bad value" for the date.

(cherry picked from commit f4d3adb)

# Conflicts:
#	src/value.cpp
  • Loading branch information
kevinbackhouse authored and mergify-bot committed Jun 26, 2021
1 parent fe83c73 commit 3c81b18
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 29 deletions.
34 changes: 17 additions & 17 deletions include/exiv2/value.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1616,93 +1616,93 @@ namespace Exiv2 {
std::string ValueType<T>::toString(long n) const
{
ok_ = true;
return Exiv2::toString<T>(value_[n]);
return Exiv2::toString<T>(value_.at(n));
}

// Default implementation
template<typename T>
long ValueType<T>::toLong(long n) const
{
ok_ = true;
return static_cast<long>(value_[n]);
return static_cast<long>(value_.at(n));
}
// #55 crash when value_[n].first == LONG_MIN
// #55 crash when value_.at(n).first == LONG_MIN
#define LARGE_INT 1000000
// Specialization for rational
template<>
inline long ValueType<Rational>::toLong(long n) const
{
ok_ = (value_[n].second != 0 && INT_MIN < value_[n].first && value_[n].first < INT_MAX );
ok_ = (value_.at(n).second != 0 && INT_MIN < value_.at(n).first && value_.at(n).first < INT_MAX );
if (!ok_) return 0;
return value_[n].first / value_[n].second;
return value_.at(n).first / value_.at(n).second;
}
// Specialization for unsigned rational
template<>
inline long ValueType<URational>::toLong(long n) const
{
ok_ = (value_[n].second != 0 && value_[n].first < LARGE_INT);
ok_ = (value_.at(n).second != 0 && value_.at(n).first < LARGE_INT);
if (!ok_) return 0;
return value_[n].first / value_[n].second;
return value_.at(n).first / value_.at(n).second;
}
// Default implementation
template<typename T>
float ValueType<T>::toFloat(long n) const
{
ok_ = true;
return static_cast<float>(value_[n]);
return static_cast<float>(value_.at(n));
}
// Specialization for rational
template<>
inline float ValueType<Rational>::toFloat(long n) const
{
ok_ = (value_[n].second != 0);
ok_ = (value_.at(n).second != 0);
if (!ok_) return 0.0f;
return static_cast<float>(value_[n].first) / value_[n].second;
return static_cast<float>(value_.at(n).first) / value_.at(n).second;
}
// Specialization for unsigned rational
template<>
inline float ValueType<URational>::toFloat(long n) const
{
ok_ = (value_[n].second != 0);
ok_ = (value_.at(n).second != 0);
if (!ok_) return 0.0f;
return static_cast<float>(value_[n].first) / value_[n].second;
return static_cast<float>(value_.at(n).first) / value_.at(n).second;
}
// Default implementation
template<typename T>
Rational ValueType<T>::toRational(long n) const
{
ok_ = true;
return Rational(value_[n], 1);
return Rational(value_.at(n), 1);
}
// Specialization for rational
template<>
inline Rational ValueType<Rational>::toRational(long n) const
{
ok_ = true;
return Rational(value_[n].first, value_[n].second);
return Rational(value_.at(n).first, value_.at(n).second);
}
// Specialization for unsigned rational
template<>
inline Rational ValueType<URational>::toRational(long n) const
{
ok_ = true;
return Rational(value_[n].first, value_[n].second);
return Rational(value_.at(n).first, value_.at(n).second);
}
// Specialization for float.
template<>
inline Rational ValueType<float>::toRational(long n) const
{
ok_ = true;
// Warning: This is a very simple conversion, see floatToRationalCast()
return floatToRationalCast(value_[n]);
return floatToRationalCast(value_.at(n));
}
// Specialization for double.
template<>
inline Rational ValueType<double>::toRational(long n) const
{
ok_ = true;
// Warning: This is a very simple conversion, see floatToRationalCast()
return floatToRationalCast(static_cast<float>(value_[n]));
return floatToRationalCast(static_cast<float>(value_.at(n)));
}

template<typename T>
Expand Down
2 changes: 1 addition & 1 deletion src/crwimage_int.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ namespace Exiv2 {
assert(pCrwMapping != 0);
ULongValue v;
v.read(ciffComponent.pData(), 8, byteOrder);
time_t t = v.value_[0];
time_t t = v.value_.at(0);
struct tm* tm = std::localtime(&t);
if (tm) {
const size_t m = 20;
Expand Down
17 changes: 16 additions & 1 deletion src/exif.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,22 @@ namespace Exiv2 {
fct = nullptr;
}
}
if ( fct ) fct(os, value(), pMetadata);
if ( fct ) {
// https://github.com/Exiv2/exiv2/issues/1706
// Sometimes the type of the value doesn't match what the
// print function expects. (The expected types are stored
// in the TagInfo tables, but they are not enforced when the
// metadata is parsed.) These type mismatches can sometimes
// cause a std::out_of_range exception to be thrown.
try {
fct(os, value(), pMetadata);
} catch (std::out_of_range&) {
os << "Bad value";
#ifdef EXIV2_DEBUG_MESSAGES
std::cerr << "Caught std::out_of_range exception in Exifdatum::write().\n";
#endif
}
}
return os;
}

Expand Down
32 changes: 22 additions & 10 deletions src/value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ namespace Exiv2 {
{
std::vector<byte>::size_type end = value_.size();
for (std::vector<byte>::size_type i = 0; i != end; ++i) {
os << static_cast<int>(value_[i]);
os << static_cast<int>(value_.at(i));
if (i < end - 1) os << " ";
}
return os;
Expand All @@ -212,27 +212,31 @@ namespace Exiv2 {
std::string DataValue::toString(long n) const
{
std::ostringstream os;
os << static_cast<int>(value_[n]);
os << static_cast<int>(value_.at(n));
ok_ = !os.fail();
return os.str();
}

long DataValue::toLong(long n) const
{
ok_ = true;
return value_[n];
return value_.at(n);
}

float DataValue::toFloat(long n) const
{
ok_ = true;
return value_[n];
return value_.at(n);
}

Rational DataValue::toRational(long n) const
{
ok_ = true;
<<<<<<< HEAD
return {value_[n], 1};
=======
return Rational(value_.at(n), 1);
>>>>>>> f4d3adbf (fix: use vector::at() rather than operator[] (#1735))
}

StringValueBase::StringValueBase(TypeId typeId)
Expand Down Expand Up @@ -296,19 +300,23 @@ namespace Exiv2 {
long StringValueBase::toLong(long n) const
{
ok_ = true;
return value_[n];
return value_.at(n);
}

float StringValueBase::toFloat(long n) const
{
ok_ = true;
return value_[n];
return value_.at(n);
}

Rational StringValueBase::toRational(long n) const
{
ok_ = true;
<<<<<<< HEAD
return {value_[n], 1};
=======
return Rational(value_.at(n), 1);
>>>>>>> f4d3adbf (fix: use vector::at() rather than operator[] (#1735))
}

StringValue::StringValue()
Expand Down Expand Up @@ -340,8 +348,12 @@ namespace Exiv2 {
{
value_ = buf;
// ensure count>0 and nul terminated # https://github.com/Exiv2/exiv2/issues/1484
<<<<<<< HEAD
if (value_.empty() || value_[value_.size() - 1] != '\0')
value_ += '\0';
=======
if (value_.size() == 0 || value_.at(value_.size()-1) != '\0') value_ += '\0';
>>>>>>> f4d3adbf (fix: use vector::at() rather than operator[] (#1735))
return 0;
}

Expand Down Expand Up @@ -740,22 +752,22 @@ namespace Exiv2 {
std::string XmpArrayValue::toString(long n) const
{
ok_ = true;
return value_[n];
return value_.at(n);
}

long XmpArrayValue::toLong(long n) const
{
return parseLong(value_[n], ok_);
return parseLong(value_.at(n), ok_);
}

float XmpArrayValue::toFloat(long n) const
{
return parseFloat(value_[n], ok_);
return parseFloat(value_.at(n), ok_);
}

Rational XmpArrayValue::toRational(long n) const
{
return parseRational(value_[n], ok_);
return parseRational(value_.at(n), ok_);
}

XmpArrayValue* XmpArrayValue::clone_() const
Expand Down
Binary file added test/data/issue_1706_poc.exv
Binary file not shown.
24 changes: 24 additions & 0 deletions tests/bugfixes/github/test_issue_1706.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# -*- coding: utf-8 -*-

from system_tests import CaseMeta, path


class InvalidDateXMP(metaclass=CaseMeta):
"""
Regression test for the bug described in:
https://github.com/Exiv2/exiv2/issues/1706
"""
url = "https://github.com/Exiv2/exiv2/issues/1706"

filename = path("$data_path/issue_1706_poc.exv")
commands = ["$exiv2 -PE $filename"]

stderr = [
"""Error: Directory Photo with 65280 entries considered invalid; not read.
"""
]
retval = [0]

def compare_stdout(self, i, command, got_stdout, expected_stdout):
# Check that it printed "Bad value" for the date.
self.assertRegex(got_stdout, "Exif.PentaxDng.Date\\s+Long\\s+1\\s+Bad value")

0 comments on commit 3c81b18

Please sign in to comment.