WAMimeType>>isBinary appears to be fatally incorrect

[timrowledge]

For most excellent reason I took a look at the profiler page for my current development stuff and bravely clicked on the 'GC Stats' link. It did not go well.

The fault is related to the 'profiler.svg' provided by WAToolFiles>>#profilerSvg. What happens (in Squeak 5.3-19435) is that the ByteArray returned is being turned into a mime document with a mimetype of 'svg+xml'. Unfortunately that is not being seen as a binary type and thus we go down the path of trying to run GRPharoUtf8CodecStream>>#nextPutAll: which sends #isByteString to the ByteArray; which dNU:.

After an afternoon poking around it appears to me that the most likely cause of the issue is in WAMimeType>>#isBinary where we see this -

isBinary
    "answers whether the contents of a document of the receiving mime type are binary"
    self main = 'text' ifTrue: [ ^ false ].
    self main = 'application'
        ifTrue: [
            "application/json is text"
            self sub = 'json' ifTrue: [ ^ false ] ].
    GRPlatform subStringsIn: self sub splitBy: $+ do: [ :each |
        "application/(x-)javascript and application/xml are text"
        (#('x-javascript' 'javascript' 'xml') includes: each)
            ifTrue: [ ^ false ] ].
    ^ true

From the comments (hey, actual comments - well done author) it looks to me that the testing of 'sub' ought to in fact be inside the ifTrue: block above.

i.e. we check for main = 'application' AND sub includes the other bits. As-is we check sub for including 'xml' even if main is not 'application'. That means that svg+xml counts as not-binary, which I strongly suspect is incorrect.

So, making the obvious trivial edits we get

isBinary
    "answers whether the contents of a document of the receiving mime type are binary"
    self main = 'text' ifTrue: [ ^ false ].
    self main = 'application'
        ifTrue: [
            "application/json is text"
            self sub = 'json' ifTrue: [ ^ false ] .
    GRPlatform subStringsIn: self sub splitBy: $+ do: [ :each |
        "application/(x-)javascript and application/xml are text"
        (#('x-javascript' 'javascript' 'xml') includes: each)
            ifTrue: [ ^ false ] ] ].
    ^ true

... and the profiler svg is properly rendered and the page load does not fail and there is no text sent to the Transcript from a background process that causes it to crash, which is what lead me to all this fun stuff.

[jbrichau]

‘Image/svg-xml’ is a text format. It’s the files that have been encoded wrong in the file library due to exactly the bug that this format was recognised as binary in the past. The file methods should have been changed while fixing the bug, which was done for others but these were forgotten.

Commit https://github.com/SeasideSt/Seaside/commit/f8bb0de8e6afb62adea3e061e263b52799d53b77 was incomplete and the same should be done for all svg files in the codebase.

[timrowledge]

Thanks Johan. That makes some sense.

I took a look at the change in commit f8bb0de and, wow, that's an interstingly done file format. I've never looked inside an svg before.

My actual problem then is that I too have a bunch of 'files' that will need converting from ByteArrays or otherwise re-creating. Can you tell me what you did for that? Or did you go back to some original artwork and export it differently?

As a work-around until you can upate the pacakage, would it be plausible to use a mimetype of 'image/svg' for my cases where the data is a ByteArray? I see 'image/png' for png files, so I'm wildly flailing for ideas...

{later} well no, image/svg isn't allowed. For now I have a hack-around that checks the document content for ByteArray in WAResponse>document: and converts to binary if required.

[jbrichau]

The ByteArray just contains the text. I don't exactly remember the piece of code I wrote to convert them back to text but I will share it here when I get at it. I'm swamped with other things right now but it's the first issue I'll tackle when getting back here.

[timrowledge]

Understood and appreciated

[gcotelli]

@timrowledge probably the best is to use deployFiles over the library with the old version and the import the files again using recursivelyAddAllFilesIn: or something like that using the new version.

[timrowledge]

Interesting idea; thanks, I'll give it a try.

tim