Issue 267 in glassdb: ZipArchiveMember>>#compressDataTo: uses unnecessary #asString

Status: Accepted
Owner: [hidden email]
Labels: Type-Defect Priority-Medium GLASS-Server Version-2.4.x  
Version-1.0-beta.8

New issue 267 by [hidden email]: ZipArchiveMember>>#compressDataTo:  
uses unnecessary #asString
http://code.google.com/p/glassdb/issues/detail?id=267

ZipArchiveMember>>#compressDataTo: uses
#asString on the byteArray it generates. This may result in
non-byte-Strings to be generated (with characters where value >255).
However, these cannot be stored in a byte array, as is subsequently done by
#nextPutAll: .
Ommiting #asString just works fine.


compressDataTo: aStream
        "Copy my deflated data to the given stream."
        | startPos endPos compressedBytes unCompressedBytes |

        startPos := aStream position.

        unCompressedBytes := (self readRawChunk: uncompressedSize) asByteArray.
        compressedBytes :=  unCompressedBytes _compressBytes.
        compressedBytes == nil
                ifTrue: [
                        "needed to undo the previous readRawChunk:"
                        self rewindData.
                        ^self stCompressDataTo: aStream ].
- aStream nextPutAll: compressedBytes asString.
+ aStream nextPutAll: compressedBytes.
        endPos := aStream position.
        compressedSize := endPos - startPos.
        crc32 := unCompressedBytes _computeCRC32: 0.

Comment #1 on issue 267 by [hidden email]:  
ZipArchiveMember>>#compressDataTo: uses unnecessary #asString
http://code.google.com/p/glassdb/issues/detail?id=267

probably relevant GemStone Stack Trace


InterpreterError 2099: Byte objects store values from 0 to 255, not <$<D3>>.
1 GRGemStonePlatform >> logError:title: @2 line 4  [GsMethod OOP 216131329]
2 WAGsSwazooAdaptor >> internalServerErrorMessage: @10 line 12  [GsMethod  
OOP 260241665]
3 ComplexBlock in GRGemStonePlatform >>  
seasideProcessRequest:adaptor:resultBlock: @29 line 20  [GsMethod OOP  
239431681]
4 ExceptionHandler >> caughtExceptionWithAction: @5 line 4  [GsMethod OOP  
11786753]
5 ComplexBlock in ExceptionHandler >> caughtEx:number:cat:args:handler: @12  
line 13  [GsMethod OOP 11786241]
6 ComplexBlock in ExecutableBlock >> ensure: @4 line 11  [GsMethod OOP  
2311681]
7 ComplexBlock in ExecutableBlock >> ensure: @6 line 11  [GsMethod OOP  
2311681]
8 ExceptionHandler >> caughtEx:number:cat:args:handler: @16 line 14  
[GsMethod OOP 11786241]
9 ComplexBlock in ExceptionHandler >> try:on:do: @10 line 12  [GsMethod OOP  
11784705]
10 Exception >> _signal:number:args: @2 line 7  [GsMethod OOP 2340865]
11 System class >> signal:args:signalDictionary: @5 line 13  [GsMethod OOP  
3747329]
12 Object >> _error:args: @7 line 10  [GsMethod OOP 1913857]
13 Object >> _error: @2 line 5  [GsMethod OOP 3998209]
14 Object >> at:put: @29 line 24  [GsMethod OOP 1916161]
15 SequenceableCollection >> add: @3 line 9  [GsMethod OOP 3911937]
16 ComplexBlock in ByteArray >> addAll: @18 line 21  [GsMethod OOP 7173377]
17 Collection >> do: @5 line 10  [GsMethod OOP 2572801]
18 ByteArray >> addAll: @19 line 21  [GsMethod OOP 7173377]
19 WriteStream >> nextPutAll: @8 line 9  [GsMethod OOP 3550465]
20 ZipArchiveMember >> compressDataTo: @13 line 14  [GsMethod OOP 19329537]
21 ZipArchiveMember >> writeDataTo: @9 line 9  [GsMethod OOP 19330817]
22 ZipArchiveMember >> writeTo: @5 line 5  [GsMethod OOP 19328513]
23 ComplexBlock in ZipArchive >> writeTo: @3 line 3  [GsMethod OOP  
211900417]
24 Collection >> do: @5 line 10  [GsMethod OOP 2572801]
25 ZipArchive >> writeTo: @5 line 2  [GsMethod OOP 211900417]
26 MCMczWriter >> flush @2 line 2  [GsMethod OOP 25973249]
27 MCMczWriter class >> fileOut:on: @5 line 5  [GsMethod OOP 27185921]
28 MCVersion >> fileOutOn: @2 line 2  [GsMethod OOP 25488641]
29 ComplexBlock in SSVersion >> buildDiffFrom:to: @24 line 14  [GsMethod  
OOP 292872961]
30 SequenceableCollection class >> streamContents: @5 line 4  [GsMethod OOP  
17928705]
31 ComplexVCBlock in SSVersion >> buildDiffFrom:to: @25 line 14  [GsMethod  
OOP 292872961]
32 GRGemStonePlatform >> squeaksourceDoWithoutProgressNotification: @1 line  
3  [GsMethod OOP 307469825]
33 SSVersion >> buildDiffFrom:to: @27 line 6  [GsMethod OOP 292872961]
34 SSVersion >> buildDiff:in: @14 line 5  [GsMethod OOP 292876033]
35 ComplexBlock in SSVersion >> getDiffyVersion:in: @11 line 7  [GsMethod  
OOP 292869889]
36 SSVersion >> getDiffyVersion:in: @12 line 8  [GsMethod OOP 292869889]
37 ComplexVCBlock in SSUrlFilter >> changes:ofProject: @16 line 13  
[GsMethod OOP 312000257]
38 ComplexBlock in SSUrlFilter >> projectNamed:do: @9 line 9  [GsMethod OOP  
293041409]
39 Object >> ifNil:ifNotNil: @1 line 4  [GsMethod OOP 17863937]
40 SSUrlFilter >> projectNamed:do: @10 line 8  [GsMethod OOP 293041409]
…

Updates:
        Labels: Milestone-1.0-beta.8.7

Comment #2 on issue 267 by [hidden email]:  
ZipArchiveMember>>#compressDataTo: uses unnecessary #asString
http://code.google.com/p/glassdb/issues/detail?id=267

(No comment was entered for this change.)

Updates:
        Labels: -Milestone-1.0-beta.8.7

Comment #3 on issue 267 by [hidden email]:  
ZipArchiveMember>>#compressDataTo: uses unnecessary #asString
http://code.google.com/p/glassdb/issues/detail?id=267

I'm disinclined to make this change, because this code is used constantly  
and is working perfectly fine....except in this one instance.

I am curious about this section of the stack:

18 ByteArray >> addAll: @19 line 21  [GsMethod OOP 7173377]
19 WriteStream >> nextPutAll: @8 line 9  [GsMethod OOP 3550465]
20 ZipArchiveMember >> compressDataTo: @13 line 14  [GsMethod OOP 19329537]

The zip code is expecting to compressDataTo: a RWBinaryOrTextStream. if a  
RWBinaryOrTextStream is used then the argument to nextPutAll: shouldn't  
matter (in fact in the GemStone implementation, the arg is always converted  
to a String, so the asString is not the problem)...

According to the stack the data is being compressed to a WriteStream...

Finally when I look at the code in SqueakSource 3.0-beta.2 (dkh.62),  
SSVersion >> buildDiffFrom:to: is using a RWBinaryOrTextStream consequently  
I think that this bug is no longer a bug...