diff --git a/src/OpenColorIO/ContextVariableUtils.cpp b/src/OpenColorIO/ContextVariableUtils.cpp index 82726d423..8340a5cd3 100644 --- a/src/OpenColorIO/ContextVariableUtils.cpp +++ b/src/OpenColorIO/ContextVariableUtils.cpp @@ -127,7 +127,10 @@ void LoadEnvironment(EnvMap & map, bool update) static std::string ResolveContextVariablesImpl(const std::string & str, const EnvMap & map, UsedEnvs & used, int depth) { - // Guard against infinite recursion from cyclic variable references. + // Guard against infinite recursion at the string substitution level (e.g. $A expands + // to $B which expands to $A). This is independent from the graph-level guards in + // CollectContextVariables() and Transform.cpp's BuildOps(), which protect against cycles + // in the color space / look / view transform reference graph. if (depth > 32) return str; // Early exit if no reserved tokens are found. @@ -176,11 +179,30 @@ std::string ResolveContextVariables(const std::string & str, const EnvMap & map, return ResolveContextVariablesImpl(str, map, used, 0); } -bool CollectContextVariables(const Config & config, +bool CollectContextVariables(const Config & config, const Context & context, ConstTransformRcPtr transform, ContextRcPtr & usedContextVars) { + // Guard against infinite recursion through cycles in the color space / look / view + // transform reference graph (e.g. a ColorSpace whose from_reference is a + // ColorSpaceTransform pointing back to itself). Distinct from the string-level depth + // check in ResolveContextVariablesImpl, which only guards cyclic context-variable + // substitution within a single string; that check fires once per finite resolveStringVar + // call here and never observes this outer graph traversal. A matching guard exists in + // BuildOps() for the op-builder traversal that runs after this one. + static thread_local int depth = 0; + if (depth > 32) + { + throw Exception("Cycle detected while collecting context variables."); + } + struct DepthGuard + { + int & d; + DepthGuard(int & d_) : d(d_) { ++d; } + ~DepthGuard() { --d; } + } guard(depth); + if(ConstColorSpaceTransformRcPtr tr = DynamicPtrCast(transform)) { if (CollectContextVariables(config, context, *tr, usedContextVars)) return true; diff --git a/src/OpenColorIO/Transform.cpp b/src/OpenColorIO/Transform.cpp index 8b2b88069..061f1f733 100755 --- a/src/OpenColorIO/Transform.cpp +++ b/src/OpenColorIO/Transform.cpp @@ -49,6 +49,27 @@ void BuildOps(OpRcPtrVec & ops, if(!transform) return; + // Guard against infinite recursion through cycles in the color space / look / view + // transform reference graph (e.g. a ColorSpace whose from_reference is a + // ColorSpaceTransform pointing back to itself). A matching guard in + // CollectContextVariables() catches the same cycles during the earlier context-variable + // scan; this one protects the op-builder traversal. Both are independent from the + // string-level depth check in ResolveContextVariablesImpl, which guards cyclic + // context-variable substitution within a single string. Having the extra guard here + // is necessary for any situations where CollectContextVariables is not called, for + // example, see Config_tests.cpp/cyclic_color_space_linearity_check. + static thread_local int depth = 0; + if (depth > 32) + { + throw Exception("Cycle detected while building ops from transform."); + } + struct DepthGuard + { + int & d; + DepthGuard(int & d_) : d(d_) { ++d; } + ~DepthGuard() { --d; } + } guard(depth); + if(ConstAllocationTransformRcPtr allocationTransform = \ DynamicPtrCast(transform)) { diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index b3d69c168..28d5e430b 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -10416,3 +10416,153 @@ OCIO_ADD_TEST(Config, interchange_attributes) } } +OCIO_ADD_TEST(Config, cyclic_color_space_transform) +{ + // Regression test for a stack overflow triggered by a self-referential + // ColorSpaceTransform (a ColorSpace whose from_reference is a ColorSpaceTransform + // that points back to itself). getProcessor() must throw a clean exception + // rather than recursing until the stack guard page is hit. + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " isdata: true\n" + " - !\n" + " name: cs1\n" + " from_scene_reference: ! {src: cs0, dst: cs1}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->getProcessor("cs0", "cs1"), + OCIO::Exception, + "Cycle detected"); +} + +OCIO_ADD_TEST(Config, cyclic_color_space_two_step) +{ + // Two-step cycle: cs1 -> cs2 -> cs1. Verify the depth guard catches indirect cycles too. + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " isdata: true\n" + " - !\n" + " name: cs1\n" + " from_scene_reference: ! {src: cs0, dst: cs2}\n" + " - !\n" + " name: cs2\n" + " from_scene_reference: ! {src: cs0, dst: cs1}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->getProcessor("cs0", "cs1"), + OCIO::Exception, + "Cycle detected"); +} + +OCIO_ADD_TEST(Config, cyclic_look_transform) +{ + // Regression test: a Look whose transform is a LookTransform that references the + // same look must not crash via stack overflow on getProcessor(). Triggered here + // by a ColorSpace whose from_scene_reference is a LookTransform invoking lookA, + // whose own transform is another LookTransform invoking lookA. + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "looks:\n" + " - !\n" + " name: lookA\n" + " process_space: cs0\n" + " transform: ! {src: cs0, dst: cs0, looks: lookA}\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " - !\n" + " name: cs1\n" + " from_scene_reference: ! {src: cs0, dst: cs0, looks: lookA}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->getProcessor("cs0", "cs1"), + OCIO::Exception, + "Cycle detected"); +} + +OCIO_ADD_TEST(Config, cyclic_color_space_linearity_check) +{ + // Regression test for the BuildOps depth guard. Config::isColorSpaceLinear() reaches + // the op-builder via Config::Impl::getProcessorWithoutCaching(), which does NOT call + // CollectContextVariables() first. So even with the CollectContextVariables guard, + // this path will still infinite-recurse on a cyclic color space unless BuildOps + // independently guards against it. + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " - !\n" + " name: cs1\n" + " from_scene_reference: ! {src: cs0, dst: cs1}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->isColorSpaceLinear("cs1", OCIO::REFERENCE_SPACE_SCENE), + OCIO::Exception, + "Cycle detected"); +} + +OCIO_ADD_TEST(Config, cyclic_display_view_transform) +{ + // Regression test: a ColorSpace whose from_scene_reference is a DisplayViewTransform + // that ultimately points back to the same ColorSpace (via the view's color space) + // must not crash via stack overflow on getProcessor(). + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "displays:\n" + " D1:\n" + " - ! {name: V1, colorspace: cs_loop}\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " - !\n" + " name: cs_loop\n" + " from_scene_reference: ! {src: cs0, display: D1, view: V1}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->getProcessor("cs0", "cs_loop"), + OCIO::Exception, + "Cycle detected"); +} + diff --git a/tests/python/ConfigTest.py b/tests/python/ConfigTest.py index 9debc3da2..6f3baff7c 100644 --- a/tests/python/ConfigTest.py +++ b/tests/python/ConfigTest.py @@ -1446,6 +1446,27 @@ def test_active__displayview_lists(self): config.addActiveView(view="v1") self.assertEqual(config.getNumActiveViews(), 1) + def test_cyclic_color_space_transform(self): + """ + Regression test: a ColorSpace whose from_scene_reference is a + ColorSpaceTransform pointing back to itself must not crash the + process via stack overflow on getProcessor(). + """ + CYCLIC_PROFILE = """ocio_profile_version: 2 +roles: + default: cs0 +colorspaces: + - ! + name: cs0 + isdata: true + - ! + name: cs1 + from_scene_reference: ! {src: cs0, dst: cs1} +""" + cfg = OCIO.Config.CreateFromStream(CYCLIC_PROFILE) + with self.assertRaises(OCIO.Exception): + cfg.getProcessor("cs0", "cs1") + class ConfigVirtualWithActiveDisplayTest(unittest.TestCase): def setUp(self):