Skip to content

Handle out of bounds error in nthIterval of FilteredTimeSeriesProperty #39238

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

Merged
merged 6 commits into from
Apr 28, 2025
Merged

Handle out of bounds error in nthIterval of FilteredTimeSeriesProperty #39238

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
94adc39
throw error if out of bounds in nthIterval
MialLewis Apr 24, 2025
4c735c3
add release note
MialLewis Apr 24, 2025
2ef33ff
update nthInterval test
MialLewis Apr 24, 2025
a034712
satisfy cppcheck - remove std::move to allow for nrvo
MialLewis Apr 24, 2025
2e86abc
cast size_t to int to resolve compiler warnings
MialLewis Apr 24, 2025
384af75
respond to review comments
MialLewis Apr 25, 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
12 changes: 10 additions & 2 deletions Framework/Kernel/src/FilteredTimeSeriesProperty.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,15 @@ template <typename TYPE> TimeInterval FilteredTimeSeriesProperty<TYPE>::nthInter
deltaT = TimeSeriesProperty<TYPE>::nthInterval(n);
} else {
this->applyFilter();
deltaT = this->m_filterIntervals[std::size_t(n)];
// Throw exception if out of bounds
if (n >= static_cast<int>(this->m_filterIntervals.size())) {
const std::string error("nthInterval(): FilteredTimeSeriesProperty '" + this->name() + "' interval " +
std::to_string(n) + " does not exist");
g_log.debug(error);
throw std::runtime_error(error);
} else {
deltaT = this->m_filterIntervals[std::size_t(n)];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do this with at(), supposed to be a safe way to access vectors, although it still throws an exception?
https://en.cppreference.com/w/cpp/container/array/at
Just letting you know about it, the code is fine as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think we want the custom error message in this case, in keeping with other error handling within the function.

}

return deltaT;
Expand Down Expand Up @@ -405,7 +413,7 @@ Kernel::TimeROI *FilteredTimeSeriesProperty<TYPE>::intersectFilterWithOther(cons
auto roi = new TimeROI(*m_filter.get());
if (other && (!other->useAll()))
roi->update_or_replace_intersection(*other);
return std::move(roi);
return roi;
}

template <typename TYPE> const Kernel::TimeROI &FilteredTimeSeriesProperty<TYPE>::getTimeROI() const {
Expand Down
6 changes: 6 additions & 0 deletions Framework/Kernel/test/LogFilterTest.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ class LogFilterTest : public CxxTest::TestSuite {
TS_ASSERT_EQUALS(p->nthInterval(4).end_str(),
"2007-Nov-30 16:17:50"); // nth interval changed to use
// previous interval now.

// test out of bounds check given filter applied
auto testFilter = createTestFilter(1);
LogFilter flt(p);
flt.addFilter(*testFilter);
TS_ASSERT_THROWS(flt.data()->nthInterval(5), const std::runtime_error &);
}

void testFilterWithTrueAtStart() {
Expand Down
1 change: 1 addition & 0 deletions docs/source/release/v6.13.0/Framework/Bugfixes/39238.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Handled out of bounds error possible when using get ``nthTime`` and ``nthInterval`` methods of a ``FilteredTimeSeriesProperty``. This occurred when the argument ``n`` was specified to be greater than the maximum index of the time filter intervals present.