[MS-ONESTORE] deserializing of optional field in ObjectSpaceObjectPropSet possibly bugged

[blu-base]

In the file ObjectSpaceObjectPropSet.cs of the ONESTORE adapter, we find the following loop:

line 49ff:

if (this.OIDs.Header.OsidStreamNotPresent == 0 && this.OIDs.Header.ExtendedStreamsPresent == 1)
            {
                this.ContextIDs = new ObjectSpaceObjectStreamOfContextIDs();
                len = this.ContextIDs.DoDeserializeFromByteArray(byteArray, index);
                index += len;
            }

Please put on your focus on this.OIDs.Header.ExtendedStreamsPresent == 1 and compare with the specification of ONESTORE

i may quote:

ContextIDs (variable): An optional ObjectSpaceObjectStreamOfContextIDs (section 2.6.4) that specifies the count and list of contexts referenced by this ObjectSpaceObjectPropSet structure. MUST be present if OSIDs is present and the value of the OSIDs.header.ExtendedStreamsPresent field is true; otherwise, the ContextIDs field MUST NOT be present. The count of referenced contexts is calculated as the number of properties specified by the body field with PropertyID equal to 0xC plus the number of referenced contexts specified by properties with PropertyID equal to 0xD, 0x10, and 0x11. This count MUST be equal to the value of ContextIDs.header.Count field. Properties that reference other contexts MUST be matched with the CompactID structures from ContextIDs.body field in the same order as the properties are listed in the body.rgPrids field.

OSIDs is present if the first condition is met (this.OIDs.Header.OsidStreamNotPresent == 0).
However, the specifiction states, that inside the OSIDs' header extendedStreamsPresent must be true. This is different from OIDs' header as currently in the code.

If the specification is strictly correct, the if condition shown above contains a bug (which can be fixed trivially).
Otherwise, the specification would have a typo.

Which location of an error is more plausible to you?

[blu-base]

In case, you agree that the specification is likely not the issue, you can find a fix in the PR #46.

[tomjebo]

@blu-base thanks for finding this discrepancy with the [MS-ONESTORE] specification 2.6.1 details. I've reviewed #46 as well and according to the specification it appears you are correct. Have you actually run this test suite (and relevant case) against a OneNote file to ensure this works?

[blu-base]

Hi @tomjebo, no i have not run the test suit.

For now, i was mostly interested in studying the onestore specification.
Moreover, i am not running any Microsoft OS for myself. That's why I didn't invest time getting it to run somewhere else, because superficially, this does not seem likely possible.

Lacking in-depth experience in the involved tooling, I can't offer a fully justified solution.
Therefore, i can only declare this discrepancy and initialize a discussion.

Are you able to test my changes?

[tomjebo]

@blu-base Thanks for the context. I'm checking with our OneNote team and doing some investigation here. It's still possible that the [MS-ONESTORE] specification might need a behavior note and that the test suite might be doing the correct thing per that behavior. I'll follow up once we have more information. I will also try to test your changes.

[tomjebo]

@blu-base I merged your #46. Thank you for helping us improve the test suites!