Profiler page 'GC Stats' problem with profiler.svg ByteArray

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.

I've added this to a new issue on github.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
My Go this  amn keyboar  oesn't have any  's.


_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

well… no

‘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.

Johan

_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

Thank you Johan; that makes perfectly good sense. Amusingly it makes my subject line a better description of the problem than my oh-so-careful analysis :-)

> On 2020-09-14, at 11:55 PM, Johan Brichau <[hidden email]> wrote:
>
> well… no
>
> ‘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.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
If at first you don't succeed, destroy all evidence that you tried.


_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev