Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
uentity opened this issue Jul 8, 2024 · 3 comments
Open

Comments

@uentity
Copy link

uentity commented Jul 8, 2024

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
Copy link
Owner

kizzx2 commented Jul 8, 2024

@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
Copy link
Author

uentity commented Jul 8, 2024

@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
Copy link
Owner

kizzx2 commented Jul 15, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants