Hi all!
I did the following:
- Filed it into a virgin 5707. Noted I need to use Andreas version and then add Ned's single method fix. Not sure why Neds changeset didn't include the Test class definition. Then when I wrote this Andreas posted a v7 fixed accordingly, fine. :) That one also filed in cleanly in 5707.
- Ran tests. All green.
- Tested producing a .zip file and opening it too. Just to see if those things broke, it worked fine. Also uses this code in my BFAV image, which does gzipping/gunzipping.
- Looked over the code changes but didn't analyze it. Noted the "Debugging" class var, but we can keep it for a while. Sidenote: Might be interesting to have a convention for such things.
- Looked at the generated PNGs in XP viewer and Firefox. Didn't check all but they worked fine.
Anyway, this is stuff from highly esteemed Squeakers, and it seems to have been tested a bit by other people too. And if we can't trust them then... :)
regards, Göran
Anyway, this is stuff from highly esteemed Squeakers, and it seems to have been tested a bit by other people too. And if we can't trust them then... :)
Trust or not - it ain't working correctly ;-( Turns out that the CRC validation is broken since a) #pastEndRead may return nil so that the crc update is incorrectly applied to the "same bytes" multiple times and b) some internal methods quite deliberately set the #position explicitly. What then happens is that perfectly valid PNGs will raise crc errors.
So CRC checking needs to be added at a lower level - namely right at the place where the data is decoded. The attached CS (which should go after V7) addresses these issues.
"Change Set: ZipCrcTests Date: 29 February 2004 Author: Andreas Raab
Integrates CRC validation at the level of InflateStream thereby avoiding some of the problems when dealing with it in subclasses. With these changes CRC validation is implicit - when we hit the end of a stream (such as upon #contents, #upToEnd or similar) the CRC will be validated automatically. If a missing or wrong crc is encountered a CRCError will be raised. Some tests are provided to show what is expected if a missing/wrong CRC is encountered. "
Cheers, - Andreas
Hi,
I'm totally confused with all the fixes related to Zip and PNG. If one of you can review/approve summarise this would help.
Stef
Anyway, this is stuff from highly esteemed Squeakers, and it seems to have been tested a bit by other people too. And if we can't trust them then... :)
Trust or not - it ain't working correctly ;-( Turns out that the CRC validation is broken since a) #pastEndRead may return nil so that the crc update is incorrectly applied to the "same bytes" multiple times and b) some internal methods quite deliberately set the #position explicitly. What then happens is that perfectly valid PNGs will raise crc errors.
So CRC checking needs to be added at a lower level - namely right at the place where the data is decoded. The attached CS (which should go after V7) addresses these issues.
"Change Set: ZipCrcTests Date: 29 February 2004 Author: Andreas Raab
Integrates CRC validation at the level of InflateStream thereby avoiding some of the problems when dealing with it in subclasses. With these changes CRC validation is implicit - when we hit the end of a stream (such as upon #contents, #upToEnd or similar) the CRC will be validated automatically. If a missing or wrong crc is encountered a CRCError will be raised. Some tests are provided to show what is expected if a missing/wrong CRC is encountered. "
squeak-dev@lists.squeakfoundation.org
-
Andreas Raab -
ducasse -
gohu＠bluefish.se