Skip to content

Commit a431bc8

Browse files
committed
fix(time-zone): a failed TryFind handed back the caller's previous zone (#2177, SR-AUD-224)
TryFindSystemTimeZoneById caught the lookup failure and returned false without touching `result`, so a caller reusing one variable across several lookups was handed the zone from an earlier, unrelated call. Measured before the fix: TryFind("Mars/Olympus", out) returned false with out still holding "UTC". Real .NET assigns null to the out parameter; the catch-all now resets it. +6 regressions in TimeZoneInfoTests.cpp covering a populated out parameter, an empty id, a path-traversal id, two consecutive failures, and both directions of the success path. Mutation proven: deleting the reset fails exactly the four pins that depend on it and leaves the two controls green. Suite 114 -> 120, all passing. One inline header body; no public signature, virtual, vtable, object layout or mangled symbol changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014joExjgZv4aWFpzADeQGZm
1 parent 886c066 commit a431bc8

3 files changed

Lines changed: 66 additions & 0 deletions

File tree

modules/time-zone/include/System/TimeZoneInfo.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,13 +564,22 @@ namespace System {
564564
* @brief Tries to find a time zone by ID; returns false instead of throwing.
565565
*
566566
* C++ counterpart of .NET TimeZoneInfo.TryFindSystemTimeZoneById(string, out TimeZoneInfo).
567+
*
568+
* On failure @p result is set to @c nullptr, mirroring .NET's assignment of @c null to the
569+
* @c out parameter. A caller that reuses one variable across several lookups therefore
570+
* cannot be handed the previous zone by a lookup that failed.
571+
*
572+
* @param id The time zone identifier to look up.
573+
* @param result Receives the zone on success and @c nullptr on failure.
574+
* @return true if the zone was found.
567575
*/
568576
static bool TryFindSystemTimeZoneById(const std::string& id,
569577
std::shared_ptr<TimeZoneInfo>& result) {
570578
try {
571579
result = FindSystemTimeZoneById(id);
572580
return true;
573581
} catch (...) {
582+
result.reset();
574583
return false;
575584
}
576585
}

modules/time-zone/tests/System/TimeZoneInfoTests.cpp

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,3 +606,60 @@ TEST(TimeZoneInfoTests, TransitionTime_GetHashCode_FloatingRule) {
606606
tod, 10, 4, System::DayOfWeek::Sunday);
607607
EXPECT_EQ(t.GetHashCode(), 10 ^ (4 << 8));
608608
}
609+
610+
// ---------------------------------------------------------------------------
611+
// Ticket #2177 (SR-AUD-224): a failed TryFindSystemTimeZoneById must clear the
612+
// caller's out parameter. Before this fix the catch-all returned false without
613+
// touching `result`, so a caller reusing one variable across several lookups was
614+
// handed the previous zone by a lookup that had failed. Measured before the fix
615+
// (build-probe/2176_probe1_surface.log): TryFind("Mars/Olympus", out) returned
616+
// false with out still holding "UTC". Real .NET assigns null to the out parameter.
617+
// ---------------------------------------------------------------------------
618+
619+
TEST(TimeZoneInfoTests, TryFind_Failure_ClearsPreviouslyPopulatedOutParameter) {
620+
std::shared_ptr<TimeZoneInfo> tz = TimeZoneInfo::FindSystemTimeZoneById("UTC");
621+
ASSERT_NE(tz, nullptr);
622+
ASSERT_EQ(tz->getIdProperty(), "UTC");
623+
EXPECT_FALSE(TimeZoneInfo::TryFindSystemTimeZoneById("Mars/Olympus", tz));
624+
EXPECT_EQ(tz, nullptr);
625+
}
626+
627+
TEST(TimeZoneInfoTests, TryFind_Failure_EmptyId_ClearsOutParameter) {
628+
std::shared_ptr<TimeZoneInfo> tz = TimeZoneInfo::FindSystemTimeZoneById("UTC");
629+
ASSERT_NE(tz, nullptr);
630+
EXPECT_FALSE(TimeZoneInfo::TryFindSystemTimeZoneById("", tz));
631+
EXPECT_EQ(tz, nullptr);
632+
}
633+
634+
TEST(TimeZoneInfoTests, TryFind_Failure_PathTraversal_ClearsOutParameter) {
635+
std::shared_ptr<TimeZoneInfo> tz = TimeZoneInfo::FindSystemTimeZoneById("UTC");
636+
ASSERT_NE(tz, nullptr);
637+
EXPECT_FALSE(TimeZoneInfo::TryFindSystemTimeZoneById("../../etc/passwd", tz));
638+
EXPECT_EQ(tz, nullptr);
639+
}
640+
641+
TEST(TimeZoneInfoTests, TryFind_Failure_AfterFailureLeavesNullNotStale) {
642+
// Two failures in a row must both leave the parameter null, not resurrect anything.
643+
std::shared_ptr<TimeZoneInfo> tz;
644+
EXPECT_FALSE(TimeZoneInfo::TryFindSystemTimeZoneById("Mars/Olympus", tz));
645+
EXPECT_EQ(tz, nullptr);
646+
EXPECT_FALSE(TimeZoneInfo::TryFindSystemTimeZoneById("Venus/Maxwell", tz));
647+
EXPECT_EQ(tz, nullptr);
648+
}
649+
650+
TEST(TimeZoneInfoTests, TryFind_Success_OverwritesAPreviousZone) {
651+
// The success path must still replace whatever was there.
652+
std::shared_ptr<TimeZoneInfo> tz = TimeZoneInfo::FindSystemTimeZoneById("Europe/Prague");
653+
ASSERT_NE(tz, nullptr);
654+
ASSERT_TRUE(TimeZoneInfo::TryFindSystemTimeZoneById("UTC", tz));
655+
ASSERT_NE(tz, nullptr);
656+
EXPECT_EQ(tz->getIdProperty(), "UTC");
657+
}
658+
659+
TEST(TimeZoneInfoTests, TryFind_SuccessThenFailure_DoesNotKeepTheSuccess) {
660+
std::shared_ptr<TimeZoneInfo> tz;
661+
ASSERT_TRUE(TimeZoneInfo::TryFindSystemTimeZoneById("Europe/Prague", tz));
662+
ASSERT_NE(tz, nullptr);
663+
EXPECT_FALSE(TimeZoneInfo::TryFindSystemTimeZoneById("Europe/Nowhere", tz));
664+
EXPECT_EQ(tz, nullptr);
665+
}

plan.sqlite3

0 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)