Base64MimeConverter class>>mimeDecodeToBytes: change breaks image load
Hi Guille & Pavel, On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)). PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ... Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails. Guille, can you explain the change. The commit log just says "make tests run". Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position. Thanks, Alistair
On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)).
PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ...
Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails.
Guille, can you explain the change. The commit log just says "make tests run".
Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position.
So doing f reset at the caller site would fix it then ? Note that there is the newer ZnBase64Encoder that can be used as follows: f := (ZnBase64Encoder new decode: self imageData) readStream. Sven
Hi Sven, On 15 March 2018 at 12:47, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)).
PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ...
Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails.
Guille, can you explain the change. The commit log just says "make tests run".
Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position.
So doing
f reset
at the caller site would fix it then ?
Yes, but I was thinking more about whether it was intended to put this responsibility on the caller (changing behaviour).
Note that there is the newer ZnBase64Encoder that can be used as follows:
f := (ZnBase64Encoder new decode: self imageData) readStream.
That works too. Thanks, Alistair
BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods: Remove String>>#base64Encoded. Change ByteArray>>#base64Encoded "Encode the receiver using Base64, see ZnBase64Encoder" ^ ZnBase64Encoder new encode: self String>>#base64Decoded "Decode the receiver using Base64, see ZnBase64Encoder" ^ ZnBase64Encoder new decode: self The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64: So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept.
On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)).
PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ...
Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails.
Guille, can you explain the change. The commit log just says "make tests run".
Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position.
So doing
f reset
at the caller site would fix it then ?
Note that there is the newer ZnBase64Encoder that can be used as follows:
f := (ZnBase64Encoder new decode: self imageData) readStream.
Sven
Also: 'apicture.png' asFileReference inspect. Will raise a "image format not recognised" because: - AbstractFileReference>>binaryReadStreamDo: ultimately uses a ZnBufferedReadStream - ImageReadWriter>>on: sends #reset to the stream and ZnBufferedReadStream doesn't implement #reset. My guess is that ZnBufferedReadStream should implement #reset (it has #position:). But I'm not familiar with the current thinking on stream rationalisation. I'll raise some fogbugz issues a bit later. Image downloaded today: Pharo 7.0 Build information: Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d (64 Bit) Thanks, Alistair On 15 March 2018 at 13:08, Sven Van Caekenberghe <sven@stfx.eu> wrote:
BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods:
Remove
String>>#base64Encoded.
Change
ByteArray>>#base64Encoded "Encode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new encode: self
String>>#base64Decoded "Decode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new decode: self
The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64:
So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept.
On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)).
PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ...
Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails.
Guille, can you explain the change. The commit log just says "make tests run".
Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position.
So doing
f reset
at the caller site would fix it then ?
Note that there is the newer ZnBase64Encoder that can be used as follows:
f := (ZnBase64Encoder new decode: self imageData) readStream.
Sven
Yeah, right, good catch. But what is this nonsense: PNGReadWriter>>on: aStream (stream := aStream) reset. stream binary "Note that 'reset' makes a file be text. Must do this after." Of course you can only read an image from a binary stream, there should be nothing else there but stream := aStream
On 15 Mar 2018, at 13:31, Alistair Grant <akgrant0710@gmail.com> wrote:
Also:
'apicture.png' asFileReference inspect.
Will raise a "image format not recognised" because:
- AbstractFileReference>>binaryReadStreamDo: ultimately uses a ZnBufferedReadStream - ImageReadWriter>>on: sends #reset to the stream
and ZnBufferedReadStream doesn't implement #reset.
My guess is that ZnBufferedReadStream should implement #reset (it has #position:). But I'm not familiar with the current thinking on stream rationalisation.
I'll raise some fogbugz issues a bit later.
Image downloaded today:
Pharo 7.0 Build information: Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d (64 Bit)
Thanks, Alistair
On 15 March 2018 at 13:08, Sven Van Caekenberghe <sven@stfx.eu> wrote:
BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods:
Remove
String>>#base64Encoded.
Change
ByteArray>>#base64Encoded "Encode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new encode: self
String>>#base64Decoded "Decode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new decode: self
The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64:
So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept.
On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)).
PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ...
Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails.
Guille, can you explain the change. The commit log just says "make tests run".
Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position.
So doing
f reset
at the caller site would fix it then ?
Note that there is the newer ZnBase64Encoder that can be used as follows:
f := (ZnBase64Encoder new decode: self imageData) readStream.
Sven
And in PNGReadWriter>>nextImage we should remove stream reset and stream binary. This probably goes for the other image readers as well.
On 15 Mar 2018, at 13:38, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Yeah, right, good catch.
But what is this nonsense:
PNGReadWriter>>on: aStream (stream := aStream) reset. stream binary "Note that 'reset' makes a file be text. Must do this after."
Of course you can only read an image from a binary stream, there should be nothing else there but
stream := aStream
On 15 Mar 2018, at 13:31, Alistair Grant <akgrant0710@gmail.com> wrote:
Also:
'apicture.png' asFileReference inspect.
Will raise a "image format not recognised" because:
- AbstractFileReference>>binaryReadStreamDo: ultimately uses a ZnBufferedReadStream - ImageReadWriter>>on: sends #reset to the stream
and ZnBufferedReadStream doesn't implement #reset.
My guess is that ZnBufferedReadStream should implement #reset (it has #position:). But I'm not familiar with the current thinking on stream rationalisation.
I'll raise some fogbugz issues a bit later.
Image downloaded today:
Pharo 7.0 Build information: Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d (64 Bit)
Thanks, Alistair
On 15 March 2018 at 13:08, Sven Van Caekenberghe <sven@stfx.eu> wrote:
BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods:
Remove
String>>#base64Encoded.
Change
ByteArray>>#base64Encoded "Encode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new encode: self
String>>#base64Decoded "Decode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new decode: self
The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64:
So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept.
On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)).
PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ...
Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails.
Guille, can you explain the change. The commit log just says "make tests run".
Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position.
So doing
f reset
at the caller site would fix it then ?
Note that there is the newer ZnBase64Encoder that can be used as follows:
f := (ZnBase64Encoder new decode: self imageData) readStream.
Sven
Yes this is that you cannot have a png embedded in stream. Just at the beginning. What funny concept. On Thu, Mar 15, 2018 at 2:14 PM, Sven Van Caekenberghe <sven@stfx.eu> wrote:
And in PNGReadWriter>>nextImage we should remove stream reset and stream binary.
This probably goes for the other image readers as well.
On 15 Mar 2018, at 13:38, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Yeah, right, good catch.
But what is this nonsense:
PNGReadWriter>>on: aStream (stream := aStream) reset. stream binary "Note that 'reset' makes a file be text. Must do this after."
Of course you can only read an image from a binary stream, there should be nothing else there but
stream := aStream
On 15 Mar 2018, at 13:31, Alistair Grant <akgrant0710@gmail.com> wrote:
Also:
'apicture.png' asFileReference inspect.
Will raise a "image format not recognised" because:
- AbstractFileReference>>binaryReadStreamDo: ultimately uses a ZnBufferedReadStream - ImageReadWriter>>on: sends #reset to the stream
and ZnBufferedReadStream doesn't implement #reset.
My guess is that ZnBufferedReadStream should implement #reset (it has #position:). But I'm not familiar with the current thinking on stream rationalisation.
I'll raise some fogbugz issues a bit later.
Image downloaded today:
Pharo 7.0 Build information: Pharo-7.0+alpha.build.696.sha.a26248bb90ee8dfba131f2f4d71ba2bbbad5d00d (64 Bit)
Thanks, Alistair
On 15 March 2018 at 13:08, Sven Van Caekenberghe <sven@stfx.eu> wrote:
BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods:
Remove
String>>#base64Encoded.
Change
ByteArray>>#base64Encoded "Encode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new encode: self
String>>#base64Decoded "Decode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new decode: self
The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64:
So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept.
On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)).
PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ...
Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails.
Guille, can you explain the change. The commit log just says "make tests run".
Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position.
So doing
f reset
at the caller site would fix it then ?
Note that there is the newer ZnBase64Encoder that can be used as follows:
f := (ZnBase64Encoder new decode: self imageData) readStream.
Sven
Hi sven can you open a bug entry? Sven I have a APi question why we could not have decodeBase64: in something else than ZnUtils. I tend to stay away from class having Util in their names :) I still have the scars on my body of my battle against Utilities. Stef On Thu, Mar 15, 2018 at 1:08 PM, Sven Van Caekenberghe <sven@stfx.eu> wrote:
BTW, for the record, we should deprecate Base64MimeConverter, and remove/change the following convenience methods:
Remove
String>>#base64Encoded.
Change
ByteArray>>#base64Encoded "Encode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new encode: self
String>>#base64Decoded "Decode the receiver using Base64, see ZnBase64Encoder"
^ ZnBase64Encoder new decode: self
The thing is, Base64 is defined as going from bytes -> characters and vice versa. Not as going from characters -> characters. For that you need to use an explicit character encoder as in ZnUtils class>>#decodeBase64: and ZnUtils>>#encodeBase64:
So yes, this would be a breaking change, but an easy one to follow since Base64 is really very simple as a concept.
On 15 Mar 2018, at 12:47, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 15 Mar 2018, at 12:28, Alistair Grant <akgrant0710@gmail.com> wrote:
Hi Guille & Pavel,
On 6 Feb Guille changed Base64MimeConverter class>>mimeDecodeToBytes: so that it doesn't reset the dataStream position back to the start of the stream. This breaks Pavel's PlayingCard, part of FreeCell (you can see that I only load important stuff :-)).
PlayingCard class>>initialize "PlayingCard initialize" | forms f | "Read the stored forms from mime-encoded data in imageData." f := Base64MimeConverter mimeDecodeToBytes: (ReadStream on: self imageData). forms := OrderedCollection new. f next = 2 ifFalse: [self error: 'corrupted imageData']. [f atEnd] whileFalse: [forms add: (Form new readFrom: f)]. ...
Previously, f would be at the start of the stream (which makes sense given this usage), but now it is at the end of the stream, so returns nil and "f next = 2" fails.
Guille, can you explain the change. The commit log just says "make tests run".
Note that a lot of users of this won't fail as they just call #contents of the stream after #mimeDecodeToBytes:, which works regardless of the current position.
So doing
f reset
at the caller site would fix it then ?
Note that there is the newer ZnBase64Encoder that can be used as follows:
f := (ZnBase64Encoder new decode: self imageData) readStream.
Sven
participants (3)
-
Alistair Grant -
Stephane Ducasse -
Sven Van Caekenberghe