search

Samuel-Tyler / fast_ber

A C++11 ASN.1 BER Encoding and Decoding Library
Boost Software License 1.0
84 stars 11 forks source link

Populate Real Type Functionality #2

Closed Samuel-Tyler closed 4 years ago

Samuel-Tyler commented 5 years ago

Real is the floating point type of ASN.1. A skeleton of the Real type has been added (include/fast_ber/ber_types/Real.hpp). Populating this skeleton will allow users to use the Real type in their ASN.1 schemas.

Encoding and decoding functionality should be provided, according to the rules specified in X.690. The class should hold the real type in partially encoded state, as with integer to reduce parsing / encoding time. Unit tests and benchmarks should be added.

Acceptance

kurkpitaine commented 4 years ago

Hello there,

Taking interest into this project as I use ASN1 everyday 👍 As I am currently implementing the Real type, I was looking to the ITU BER specs and it seems that the document you linked above is not the lastest release. The link is refering to an older one which is deprecated. The correct X690 spec is this one.

Samuel-Tyler commented 4 years ago

Thank you for taking the time to do so!

I look forward to your contribution. Please make a pull request into master branch when your submission is ready. (Only as devel is currently not building on all platforms, which I will be resolving).

I am glad to hear of your experience with ASN.1 and BER. Have you had the opportunity to run any of your specs through the compiler? If so I'd love to hear of any compilation errors.

Thank you, Sam

Samuel-Tyler commented 4 years ago

Hey @kurkpitaine, did you make any progress on this?

kurkpitaine commented 4 years ago

Hi @Samuel-Tyler ! Yep I made progress. Full spec decoding is ready, encoding is almost done. I need to finish populating all the functions and write some tests

Samuel-Tyler commented 4 years ago

Thanks, sounds great!

For encoding, no need to support all the possible encodings, pick the one that makes the most sense.

Let me submit some (failing) unit tests to show what I'm expecting.

If you would like, you may submit your work in progress for review.

Samuel-Tyler commented 4 years ago

I've added some initial test cases here:

https://github.com/Samuel-Tyler/fast_ber/pull/19/files

Hope this helps. Feel free to expand them of course.

kurkpitaine commented 4 years ago

Have finished Base 2 encoding, I open a PR for review

kurkpitaine commented 4 years ago

@Samuel-Tyler I opened a PR with my work here: https://github.com/Samuel-Tyler/fast_ber/pull/20

Samuel-Tyler commented 4 years ago

Looks very good on first inspection! Thank you very much. I will do a full review later.

I am aware that the standard implementation of std::regex is poor. We could potentially use CTRE to speed this up, if people happen to use the REAL type commonly in their codebase.

kurkpitaine commented 4 years ago

std::regex is used only if we decode an Iso 6093 NR1/NR2/NR3 encoded real number. Most of the time, it is a binary base encoding. But indeed this would be better with a better regex implementation 😊

Samuel-Tyler commented 4 years ago

@kurkpitaine I would be interested to hear more about your usage of ASN.1. What do you use it for? Are there other features you need in fast_ber for your use case?

Thanks

kurkpitaine commented 4 years ago

I use ASN.1 for ITS communications protocols, syntaxes are available here: https://forge.etsi.org/rep/ITS/ITS_ASN1 Unaligned PER encoding is exclusively used in upper layer messages. However, Canonical OER and DER encodings are used on the security layer and this is where I need an high speed ASN.1 implementation. I did not found any suitable C++ implementation which is satisfying: partial implementation, no active maintenance, ... After some research, I found fast_ber and it sounds promising !
Since BER/DER/COER are not so different, I think we can implement them into fast_ber

Samuel-Tyler commented 4 years ago

Yes, adding support for the different encodings is a priority for me.

By default, everything should be encoded as DER, currently.

Samuel-Tyler commented 4 years ago

Have you attempted to parse any schemas?

Let me know if you've seen any errors in the compiler.

Samuel-Tyler commented 4 years ago

Hey @kurkpitaine

I hope you are well during these times.

I would like to do a release soon, and I would love to include your work on the REAL type. Do you have any more commits that we can work together to get merged?

Thanks, Sam

kurkpitaine commented 4 years ago

Hi @Samuel-Tyler,

I'm good thank you, hope everything is ok on your side ! Yes, I have worked on my side, I still need to finish some tests cases but I can push my code so you can take a look at it.

Samuel-Tyler commented 4 years ago

Good to hear you are well.

That would be great if you could push the changes, I hope the merge will not be too painful as I have made a number of changes.

Thanks.

kurkpitaine commented 4 years ago

I pushed the work on my side, changes are available in the same PR.

Samuel-Tyler commented 4 years ago

Thanks! Would you like me to merge the latest changes into your branch?

kurkpitaine commented 4 years ago

Yes for sure !

Samuel-Tyler commented 4 years ago

Great, I've done so, I've also added Real to the AllType test and to the benchmarks, the results there look good.

I've left a few comments... once these are resolved we can get this merged.

kurkpitaine commented 4 years ago

Hi @Samuel-Tyler, I made changes today regarding the PR, I let you check!

Samuel-Tyler commented 4 years ago

Ok, it's ready to merge. Great work, thanks again!

kurkpitaine commented 4 years ago

I merged the PR, we can close this issue 👍