From 4b3ae40a42fd6af9f281e2f46b6e3f16feda3afc Mon Sep 17 00:00:00 2001 From: Steve Lhomme Date: Fri, 1 Mar 2024 16:01:02 +0100 Subject: [PATCH 1/5] simplify skipping data of a known child in unknown size master elements We should be skipping data with the actual EbmlSemanticContext of that element. --- src/EbmlElement.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/EbmlElement.cpp b/src/EbmlElement.cpp index 5526ac81..95c427bc 100644 --- a/src/EbmlElement.cpp +++ b/src/EbmlElement.cpp @@ -392,7 +392,8 @@ EbmlElement * EbmlElement::SkipData(EbmlStream & DataStream, const EbmlSemanticC for (EltIndex = 0; EltIndex < EBML_CTX_SIZE(Context); EltIndex++) { if (EbmlId(*Result) == EBML_CTX_IDX_ID(Context,EltIndex)) { // skip the data with its own context - Result = Result->SkipData(DataStream, EBML_SEM_CONTEXT(EBML_CTX_IDX(Context,EltIndex)), nullptr); + assert(&EBML_SEM_CONTEXT(EBML_CTX_IDX(Context,EltIndex)) == &EBML_CONTEXT(Result)); + Result = Result->SkipData(DataStream, EBML_CONTEXT(Result), nullptr); break; // let's go to the next ID } } From 23687848361b062270c13678c985d36e39ccb56f Mon Sep 17 00:00:00 2001 From: Steve Lhomme Date: Fri, 1 Mar 2024 16:03:35 +0100 Subject: [PATCH 2/5] use regular break to exit loop No other code was executed after bEndFound was set to true to exist the loop. --- src/EbmlElement.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/EbmlElement.cpp b/src/EbmlElement.cpp index 95c427bc..b5ba0158 100644 --- a/src/EbmlElement.cpp +++ b/src/EbmlElement.cpp @@ -373,8 +373,7 @@ EbmlElement * EbmlElement::SkipData(EbmlStream & DataStream, const EbmlSemanticC ///////////////////////////////////////////////// // read elements until an upper element is found ///////////////////////////////////////////////// - bool bEndFound = false; - while (!bEndFound && Result == nullptr) { + while (Result == nullptr) { // read an element /// \todo 0xFF... and true should be configurable // EbmlElement * NewElt; @@ -386,7 +385,9 @@ EbmlElement * EbmlElement::SkipData(EbmlStream & DataStream, const EbmlSemanticC TestReadElt = nullptr; } - if (Result != nullptr) { + if (Result == nullptr) + break; + unsigned int EltIndex; // data known in this Master's context for (EltIndex = 0; EltIndex < EBML_CTX_SIZE(Context); EltIndex++) { @@ -406,13 +407,10 @@ EbmlElement * EbmlElement::SkipData(EbmlStream & DataStream, const EbmlSemanticC if (Context != Context.GetGlobalContext()) { Result = SkipData(DataStream, Context.GetGlobalContext(), Result); } else { - bEndFound = true; + break; } } } - } else { - bEndFound = true; - } } } return Result; From 934d95bcade2ea0b17ca322b631a611eaa3136f5 Mon Sep 17 00:00:00 2001 From: Steve Lhomme Date: Fri, 1 Mar 2024 16:08:22 +0100 Subject: [PATCH 3/5] add a comment and reduce indentation We may find an upper level element in an unknown length master element. We skip its data as well, until there's nothing to skip. This is very suspicious. --- src/EbmlElement.cpp | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/EbmlElement.cpp b/src/EbmlElement.cpp index b5ba0158..8096f294 100644 --- a/src/EbmlElement.cpp +++ b/src/EbmlElement.cpp @@ -388,29 +388,31 @@ EbmlElement * EbmlElement::SkipData(EbmlStream & DataStream, const EbmlSemanticC if (Result == nullptr) break; - unsigned int EltIndex; - // data known in this Master's context - for (EltIndex = 0; EltIndex < EBML_CTX_SIZE(Context); EltIndex++) { - if (EbmlId(*Result) == EBML_CTX_IDX_ID(Context,EltIndex)) { - // skip the data with its own context - assert(&EBML_SEM_CONTEXT(EBML_CTX_IDX(Context,EltIndex)) == &EBML_CONTEXT(Result)); - Result = Result->SkipData(DataStream, EBML_CONTEXT(Result), nullptr); - break; // let's go to the next ID - } + unsigned int EltIndex; + // data known in this Master's context + for (EltIndex = 0; EltIndex < EBML_CTX_SIZE(Context); EltIndex++) { + if (EbmlId(*Result) == EBML_CTX_IDX_ID(Context,EltIndex)) { + // skip the data with its own context + assert(&EBML_SEM_CONTEXT(EBML_CTX_IDX(Context,EltIndex)) == &EBML_CONTEXT(Result)); + Result = Result->SkipData(DataStream, EBML_CONTEXT(Result), nullptr); + break; // let's go to the next ID } + } - if (EltIndex >= EBML_CTX_SIZE(Context)) { - if (EBML_CTX_PARENT(Context) != nullptr) { - Result = SkipData(DataStream, *EBML_CTX_PARENT(Context), Result); + if (EltIndex >= EBML_CTX_SIZE(Context)) { + if (EBML_CTX_PARENT(Context) != nullptr) { + // the element found is a parent of the original provided Context + // skip all its data as well (?!!!) until there's nothing to skip at that level + Result = SkipData(DataStream, *EBML_CTX_PARENT(Context), Result); + } else { + assert(Context.GetGlobalContext != nullptr); + if (Context != Context.GetGlobalContext()) { + Result = SkipData(DataStream, Context.GetGlobalContext(), Result); } else { - assert(Context.GetGlobalContext != nullptr); - if (Context != Context.GetGlobalContext()) { - Result = SkipData(DataStream, Context.GetGlobalContext(), Result); - } else { - break; - } + break; } } + } } } return Result; From 67eaf75c4e5d7e2c60572193eb8b4f7e261fae1d Mon Sep 17 00:00:00 2001 From: Steve Lhomme Date: Fri, 1 Mar 2024 16:17:41 +0100 Subject: [PATCH 4/5] skip global elements with their own context We don't need to use the global context that contains the list of global elements. The global element cannot be a have an unknown size and will just skip the data and return nullptr. --- src/EbmlElement.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/EbmlElement.cpp b/src/EbmlElement.cpp index 8096f294..ffde88c5 100644 --- a/src/EbmlElement.cpp +++ b/src/EbmlElement.cpp @@ -405,12 +405,8 @@ EbmlElement * EbmlElement::SkipData(EbmlStream & DataStream, const EbmlSemanticC // skip all its data as well (?!!!) until there's nothing to skip at that level Result = SkipData(DataStream, *EBML_CTX_PARENT(Context), Result); } else { - assert(Context.GetGlobalContext != nullptr); - if (Context != Context.GetGlobalContext()) { - Result = SkipData(DataStream, Context.GetGlobalContext(), Result); - } else { - break; - } + // we found a global element + Result = SkipData(DataStream, EBML_CONTEXT(Result), Result); } } } From 0f0314c020dc5371ce26987050e5b1569c7d76ba Mon Sep 17 00:00:00 2001 From: Steve Lhomme Date: Fri, 1 Mar 2024 15:39:12 +0100 Subject: [PATCH 5/5] make the global context non recursive Either the global element needs to be found in the list of elements, or though the callback. If the extra callback points to itself we may loop recursively on it. --- src/EbmlContexts.cpp | 2 +- src/EbmlElement.cpp | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/EbmlContexts.cpp b/src/EbmlContexts.cpp index 0264e274..24569fc0 100644 --- a/src/EbmlContexts.cpp +++ b/src/EbmlContexts.cpp @@ -16,7 +16,7 @@ DEFINE_SEMANTIC_ITEM(false, false, EbmlCrc32) DEFINE_SEMANTIC_ITEM(false, false, EbmlVoid) DEFINE_END_SEMANTIC(EbmlGlobal) -static DEFINE_xxx_CONTEXT(EbmlGlobal, GetEbmlGlobal_Context) +static DEFINE_xxx_CONTEXT(EbmlGlobal, nullptr) const EbmlSemanticContext & GetEbmlGlobal_Context() { diff --git a/src/EbmlElement.cpp b/src/EbmlElement.cpp index ffde88c5..36983a2e 100644 --- a/src/EbmlElement.cpp +++ b/src/EbmlElement.cpp @@ -432,8 +432,11 @@ EbmlElement *EbmlElement::CreateElementUsingContext(const EbmlId & aID, const Eb } } + if (Context.GetGlobalContext == nullptr) + // this is a already the global EbmlSemanticContext, if it's the last one and we + // didn't find our global elements, we won't find anything anymore + return nullptr; // global elements - assert(Context.GetGlobalContext != nullptr); // global should always exist, at least the EBML ones const auto& tstContext = Context.GetGlobalContext(); if (tstContext != Context) { LowLevel--;