kizzx2 / sbe-python

Easy-to-use Python SBE decoder and encoder
https://pypi.org/project/sbe
MIT License
18 stars 15 forks source link

Structure offset is counted twice in `WrappedComposite.get_raw_pointer()` #22

Open uentity opened 2 months ago

uentity commented 2 months ago

If you look at how composite fields parsing (from schema) algorithm is implemented, you will see that offsets of fields include the overall structure offset WrappedComposite.offset.

Then, get_raw_pointer(self, key) does the following:

p = self.pointers[key]
p.enum = None
return Pointer(self.offset + p.offset, p.value, p.size)

I.e. it adds the self.offset once again resulting in wrong pointer placement.

I fixed this by simply removing the self.offset term and this change fixed my data parsing pipeline. Not sure if similar fix is required in WrappedComposite.__getitem__().

kizzx2 commented 2 months ago

@uentity Thanks for the report -- do you think it'd be possible to share a sample of the data you are working on so that this can be added into test case (public)? I haven't needed to work with FIX data day-to-day for a while so it would be better for the project long-term to accumulate some test cases

You can use the library itself to decode, anonymize and then encode them again 😂

If that's OK for you thanks in advance!

uentity commented 2 months ago

@kizzx2 I understand and totally support your intention to obtain test data, but the thing is that I also don't work with FIX atm =)

We are using our own binary serialization format and adding experimental support for encoding structs with with SBE. Currently, I work on parser part and play with test schemas/data generated by reference Java SBE encoder. Data is not secret at all and I can probably try to extract binary representation of one or several messages if it would make sense, but I doubt it, because it's dummy and not from some "real" production system.

BTW, I have added several missing features to the library, like support for ref tags within composite types and similar support for Enums/Sets defined inside Composite, just to get things done (otherwise, my example schemas generated parsing errors). I don't know if you have plans to implement that, but overall it was pretty obvious and required relatively small amount of code.

kizzx2 commented 2 months ago

@uentity That sounds great -- could you please submit a PR please?