Adsk Contrib - Avoid possible cycles in transforms

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<const ColorSpaceTransform>(transform))
     {
         if (CollectContextVariables(config, context, *tr, usedContextVars)) return true;

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<const AllocationTransform>(transform))
     {

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"
+        "  - !<ColorSpace>\n"
+        "    name: cs0\n"
+        "    isdata: true\n"
+        "  - !<ColorSpace>\n"
+        "    name: cs1\n"
+        "    from_scene_reference: !<ColorSpaceTransform> {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"
+        "  - !<ColorSpace>\n"
+        "    name: cs0\n"
+        "    isdata: true\n"
+        "  - !<ColorSpace>\n"
+        "    name: cs1\n"
+        "    from_scene_reference: !<ColorSpaceTransform> {src: cs0, dst: cs2}\n"
+        "  - !<ColorSpace>\n"
+        "    name: cs2\n"
+        "    from_scene_reference: !<ColorSpaceTransform> {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"
+        "  - !<Look>\n"
+        "    name: lookA\n"
+        "    process_space: cs0\n"
+        "    transform: !<LookTransform> {src: cs0, dst: cs0, looks: lookA}\n"
+        "colorspaces:\n"
+        "  - !<ColorSpace>\n"
+        "    name: cs0\n"
+        "  - !<ColorSpace>\n"
+        "    name: cs1\n"
+        "    from_scene_reference: !<LookTransform> {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"
+        "  - !<ColorSpace>\n"
+        "    name: cs0\n"
+        "  - !<ColorSpace>\n"
+        "    name: cs1\n"
+        "    from_scene_reference: !<ColorSpaceTransform> {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"
+        "    - !<View> {name: V1, colorspace: cs_loop}\n"
+        "colorspaces:\n"
+        "  - !<ColorSpace>\n"
+        "    name: cs0\n"
+        "  - !<ColorSpace>\n"
+        "    name: cs_loop\n"
+        "    from_scene_reference: !<DisplayViewTransform> {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");
+}
+

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:
+  - !<ColorSpace>
+    name: cs0
+    isdata: true
+  - !<ColorSpace>
+    name: cs1
+    from_scene_reference: !<ColorSpaceTransform> {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):