zip bug [IMPORTANT] probably related to the recent Zip changes

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

zip bug [IMPORTANT] probably related to the recent Zip changes

Stéphane Ducasse
Sean could you have a look?

Because when I tried to click on Balloon-CamilloBruni.85.mcz in PharoInbox

I get a month as 13

asDateFromZip
        "See Date>>zipped comment"

        | yearsSinceDosEpoch month day year |
        yearsSinceDosEpoch := self >> 9. "High 7 bits"
        month := self >> 5 & 2r1111. "Middle 4 bits"
        day := self & 2r11111. "Low 5 bits"
        year := DateAndTime dosEpoch year + yearsSinceDosEpoch.
        ^ Date year: year month: month day: day.

Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Stéphane Ducasse
Probably related to

Name: Compression-SeanDeNigris.97
Author: SeanDeNigris
Time: 21 May 2012, 4:19:27.939 pm
UUID: 4936c6ef-dc79-4a72-a4c7-13b8a6840d04
Ancestors: Compression-MarcusDenker.95

* Fix conversion to/from DateAndTime and DOS timestamps. DOS format per http://blogs.msdn.com/b/oldnewthing/archive/2003/09/05/54806.aspx and http://mindprod.com/jgloss/zip.html
* Update tests, removing #testAsDosTimeIndependentOfTimezone (they are not independent; DOS timestamps are in local time)
* Renamed DateAndTime>>asDosTime to #asDosTimestamp (it is not a time, but an encoded date/time)
* Renamed DateAndTime class>>fromDosSeconds: to #fromDosTimestamp (it is not a number of seconds, like a UNIX or Squeak date/time)
* Updated ZipArchiveMember to use new method names


On Jun 10, 2012, at 11:03 PM, Stéphane Ducasse wrote:

> Sean could you have a look?
>
> Because when I tried to click on Balloon-CamilloBruni.85.mcz in PharoInbox
>
> I get a month as 13
>
> asDateFromZip
> "See Date>>zipped comment"
>
> | yearsSinceDosEpoch month day year |
> yearsSinceDosEpoch := self >> 9. "High 7 bits"
> month := self >> 5 & 2r1111. "Middle 4 bits"
> day := self & 2r11111. "Low 5 bits"
> year := DateAndTime dosEpoch year + yearsSinceDosEpoch.
> ^ Date year: year month: month day: day.
>


Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Sean P. DeNigris
Administrator
In reply to this post by Stéphane Ducasse
Stéphane Ducasse wrote
Sean could you have a look?

Because when I tried to click on Balloon-CamilloBruni.85.mcz in PharoInbox

I get a month as 13
I'll take a look. I'm very confident about that code, because I tested it over and over with real data from different sources. Although, it makes sense that, if we've been saving files with wrong values and loading them using the same formula, we wouldn't know they were wrong until we fix the format... If that's the case, maybe wrap the unzip with a check that defaults to the old format if the date is invalid or strange (e.g. year 1962)...

Sean
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Sean P. DeNigris
Administrator
In reply to this post by Stéphane Ducasse
Stéphane Ducasse wrote
Sean could you have a look?
I can't get latest 2.0 bc Jenkins is down. Can you send me an image? Otherwise I'll load the slice into 20118

Stéphane Ducasse wrote
Because when I tried to click on Balloon-CamilloBruni.85.mcz in PharoInbox
Where did you click on it? I need steps to reproduce...
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Sean P. DeNigris
Administrator
Sean P. DeNigris wrote
Where did you click on it? I need steps to reproduce...
Okay, I tried to merge it and got the error when the package was being read...

I did some experiments...
* create a zip at Mac command line -> new Pharo code correctly parses date/times
* unzip Balloon-CamilloBruni.85.mcz at Mac command line -> Mac reports date/time as 1/23/2010... I'm assuming this is wrong because pre-fix, Pharo reported 29 September 2011

So, I think the code is correct, but transitioning is a problem... let me see write up a fallback defaulting to the old date format for invalid dates...
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Sean P. DeNigris
Administrator
Sean P. DeNigris wrote
let me see write up a fallback defaulting to the old date format for invalid dates...
Damn!! I'm screwed because the bug we found yesterday in #ensureDirectory (in the VM I think) makes all my tests fail, so I'm refactoring blind!!
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Sean P. DeNigris
Administrator
Okay, here is an issue:
Issue 6054: Error when reading zip files with Squeak date/time format
http://code.google.com/p/pharo/issues/detail?id=6054

with a fix attached and a comment pointing back here:
Attached are two cs files that have a temporary fix enabling us to load mcz and other zip files, even if they have the old, incorrect date format. They should be refactored and there is a larger question to discuss [1], but they work under manual testing.

The new behavior when reading zip file timestamps is:
* parse as a DOS timestamp
* if an error is encountered, fall back and treat it like a Squeak (i.e. incorrect) DOS timestamp. This may return an incorrect result, but will not cause an error

The changes were tested by hand on Balloon-CamilloBruni.85.mcz, which Steph was having a problem with because there was 13 as a month index, which blew up [1]
The larger issue I was thinking about is: should we extend this fallback to all DOS timestamp parsing, but now that I think about it:
* I imagine there's not much of that going on besides zipping, lol
* I think it'd be better to signal an InvalidDosTimestamp, describing the change, and pointing users to how to convert their data, instead of silently trying to convert

What do you think?

Cheers,
Sean
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Ben Coman
In reply to this post by Sean P. DeNigris
Sean P. DeNigris wrote:

> Sean P. DeNigris wrote
>  
>> Where did you click on it? I need steps to reproduce...
>>
>>    
>
> Okay, I tried to merge it and got the error when the package was being
> read...
>
> I did some experiments...
> * create a zip at Mac command line -> new Pharo code correctly parses
> date/times
> * unzip Balloon-CamilloBruni.85.mcz at Mac command line -> Mac reports
> date/time as 1/23/2010... I'm assuming this is wrong because pre-fix, Pharo
> reported 29 September 2011
>
> So, I think the code is correct, but transitioning is a problem... let me
> see write up a fallback defaulting to the old date format for invalid
> dates...
>
> --
> View this message in context: http://forum.world.st/zip-bug-IMPORTANT-probably-related-to-the-recent-Zip-changes-tp4634224p4634247.html
> Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
>
>
>  
What do you get if you unzip it outside Pharo? from the operating system?

Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Sean P. DeNigris
Administrator
Ben Coman wrote
What do you get if you unzip it outside Pharo? from the operating system?
Sean P. DeNigris wrote
* unzip Balloon-CamilloBruni.85.mcz at Mac command line -> Mac reports date/time as 1/23/2010... I'm assuming this is wrong because pre-fix, Pharo reported 29 September 2011
So Mac's zip came up with a valid, but bogus date; while Pharo raised an error...
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Stéphane Ducasse
In reply to this post by Sean P. DeNigris

On Jun 11, 2012, at 3:47 AM, Sean P. DeNigris wrote:

> so I'm refactoring blind!!

Scary :)
Yes I know that feeling….


Reply | Threaded
Open this post in threaded view
|

Re: zip bug [IMPORTANT] probably related to the recent Zip changes

Peter Hugosson-Miller
In reply to this post by Sean P. DeNigris


On Mon, Jun 11, 2012 at 2:58 AM, Sean P. DeNigris <[hidden email]> wrote:

... If that's the case, maybe wrap the unzip with a check that
defaults to the old format if the date is invalid or strange (e.g. year
1962)...

What's strange about 1962? I happen to think it's a rather nice year ;-)

--
Cheers,
Peter