SeasideSt / Seaside

The framework for developing sophisticated web applications in Smalltalk.
MIT License
519 stars 71 forks source link

Squeak WAVersionUploader>>#newVersion: needs to make sure the RWBinaryOrTextStream is ascii #1391

Open timrowledge opened 1 year ago

timrowledge commented 1 year ago

WAVersionUploader>>#newVersion: uses an RWBinaryOrTextStream that gets set to binary mode when the MCZ data is written to it. That breaks the response #nextPutAll: when we get to GRPharoUtf8CodecStream>>#nextPutAll: because that expects some form of String and sends #isByteString. ByteArrays do not implement tht and so we get a dNU.

Adding an explicit #ascii near the end of the method forces the stream to non-binary mode and the response is happy.

WAVersionUploader-newVersion.txt

jbrichau commented 1 year ago

I’m not using Squeak, but I did make some changes for it in the past and managed to publish to filetree and hence to make a GitHub PR for the code contribution. Since I notice Max doing this for the other Squeak-related issues, is there no tutorial/walkthrough/… to guide Squeakers to publish a PR?

Johan

On 9 Nov 2023, at 01:55, tim Rowledge @.***> wrote:

WAVersionUploader>>#newVersion: uses an RWBinaryOrTextStream that gets set to binary mode when the MCZ data is written to it. That breaks the response #nextPutAll: when we get to GRPharoUtf8CodecStream>>#nextPutAll: because that expects some form of String and sends #isByteString. ByteArrays do not implement tht and so we get a dNU.

Adding an explicit #ascii near the end of the method forces the stream to non-binary mode and the response is happy.

WAVersionUploader-newVersion.txt https://github.com/SeasideSt/Seaside/files/13302600/WAVersionUploader-newVersion.txt — Reply to this email directly, view it on GitHub https://github.com/SeasideSt/Seaside/issues/1391, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHDZZVGTH7WLYCJCIUHEITYDQSYPAVCNFSM6AAAAAA7DYBEZWVHI2DSMVQWIX3LMV43ASLTON2WKOZRHE4DINRTGM2TONA. You are receiving this because you are subscribed to this thread.

jbrichau commented 1 year ago

Also, I'm not sure that is the way to go since a 'Pharo-specific' class is being used in Squeak. It might make sense to check fix the proper stream abstraction in Grease. But... I just read the issue, not the code solution atm.