Open branch, to fix multiple crashes on repeated zip archives reading… #200
Conversation
|
Thanks! I think I see what the changes are but can you make the spacing consistent? Also, other minor things. |
| @@ -1,44 +1,44 @@ | |||
| | |||
| Microsoft Visual Studio Solution File, Format Version 12.00 | |||
There was a problem hiding this comment.
Can you please clarify, what newline character do you use?
There was a problem hiding this comment.
Maybe it's still left over from before I tried to normalize endings but I have gitattributes set to auto.
Question is: why is this file changing at all?
There was a problem hiding this comment.
The solution file was regenerated by Visual Studio. I don't see any real changes.
There was a problem hiding this comment.
So revert it then please. Whitespace changes make a lot of noise for reviewing.
There was a problem hiding this comment.
Sorry i can't revert this file because my Git didn't see any changes.
| //only read 10 less because the last ten are auth bytes | ||
| return new WinzipAesCryptoStream(plainStream, Header.WinzipAesEncryptionData, Header.CompressedSize - 10); | ||
| } | ||
| return Header.WinzipAesEncryptionData != null ? new WinzipAesCryptoStream(plainStream, Header.WinzipAesEncryptionData, Header.CompressedSize - 10) : plainStream; |
There was a problem hiding this comment.
Why do this?
I hate the tertiary operator. It's not readable on a single line here.
| @@ -65,9 +65,28 @@ protected byte[] EncodeString(string str) | |||
|
|
|||
| internal List<ExtraData> Extra { get; set; } | |||
|
|
|||
| internal PkwareTraditionalEncryptionData PkwareTraditionalEncryptionData { get; set; } | |||
| //internal PkwareTraditionalEncryptionData PkwareTraditionalEncryptionData { get; set; } | |||
There was a problem hiding this comment.
Don't leave commented out code, please.
|
Fix for #197 I think. |
|
Please revert all the whitespace changes. You've spaced in way too far on some files for no reason. |
| #else | ||
| && (Header.PkwareTraditionalEncryptionData != null)) | ||
| #endif | ||
| if ((Header.CompressedSize == 0) && !string.IsNullOrEmpty(Header.Password)) |
There was a problem hiding this comment.
This doesn't make sense. The lack of a password should be it's own if statement. Not ANDed with this check.
| @@ -165,20 +162,16 @@ private void LoadHeader(ZipFileEntry entryHeader, Stream stream) | |||
| switch (mode) | |||
| { | |||
| case StreamingMode.Seekable: | |||
| { | |||
There was a problem hiding this comment.
don't remove braces please. They're there for a reason.
| #else | ||
| && (Header.PkwareTraditionalEncryptionData != null)) | ||
| #endif | ||
| if (Header.CompressedSize == 0) |
There was a problem hiding this comment.
I guess this is my bad while this is busting things.
I can't think through the logic at the moment but the null checks above where used to see if encryption was being attempted.
I guess that's what you were doing with the password being specified. Sorry, I guess that needs to be put back as I misread.
There was a problem hiding this comment.
I think that define that file is encrypted can be checked by using Header.Flags. I think it easiest way, because in previous code variant, encryptor in HederFactory is created based on this flag.
…. Added fix.