-
Notifications
You must be signed in to change notification settings - Fork 977
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
Experiment with using ITaggedData for AES extra data handling #470
base: master
Are you sure you want to change the base?
Conversation
I am currently going through the ExtraFields/ITaggedData for |
it was initially just to see if it's a reasonable approach to sharing the AES extra data code (rather than just having some other helper functions), rather than a concrete thing, so no problem. (as far as the shape of the api goes, see my comment about possibly not being terribly efficient, though I don't know what the scope is for changing the interface vs. public API breaks) |
44da647
to
9bd70bd
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #470 +/- ##
==========================================
+ Coverage 73.07% 73.40% +0.33%
==========================================
Files 68 68
Lines 8305 8322 +17
==========================================
+ Hits 6069 6109 +40
+ Misses 2236 2213 -23 ☔ View full report in Codecov by Sentry. |
Just trying to remember what state this was left in... Any thoughts on the type of |
I think it should definitely be |
This is an experiment to use the ITaggedData machinery for handling the AES extra data, too see how it might effect things, and to think if it might be useful for #443 (to have the extra data handling in a central place).
Notes:
It changes the TagID property on ITaggedData from
short
toushort
to accomodate the 0x9901 value for the AES extra data. Some of the other apis use int rather than short so that might be more consistent, not sure if other places could be changed to keep it working withshort
though.Not sure if using ZipHelperStream to unpack the data instead of reading array values is more extra overhead than would be ideal, though i think the read functions in
ZipExtraData
might already do more size checks than they really need to, so perhaps it could be tuned in general?Similar on the
GetData()
side - building the fixed 7 byte array with a temporary stream and 7 calls to write byte is perhaps not the optimal approach there.I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.