missing size parameter in iterm's protocol

[jerch]

timg does not set the size for iterm's IIP sequence here https://github.com/hzeller/timg/blob/31f22a46e8ae0f9c9f83e2b5da8579e0d106566f/src/iterm2-canvas.cc#L57

Following the sequence description on https://iterm2.com/documentation-images.html the size parameter is not optional (seems optional parameters are explicitly marked as such).

[hzeller]

Should be easy to add, we have the png_size value available right there to include in the sprintf().

The images worked in the tested iterm2, wezterm and konsole terminal; if you have a terminal where you observed this to be not working, it might be useful to add that to the set of terminals to test with.

[hzeller]

just tested with sending bogus values to konsole and wezterm and it seems to work fine, i.e. they don't look at that value.

I don't have an iterm2 to test as it is not available on linux. The value is ambiguous: does it mean the raw size or the base64 encoded size ?

[jerch]

Yeah the parameter is a bit unclear - since it says "file size in bytes" I'd assume it means the real file size, not the size of the base64-encoded content.

Btw vscode's integrated terminal checks against this value to spot data errors and to limit the sequence size. I find it kinda weird that other terminals just let things pass here, smells like an potential DOS vector to me.

[hzeller]

so I can test it with vscode ? What do I need to do to reproduce ?

[jerch]

You'd need vscode 1.80+ version (got recently added), and make sure, that the terminal image support is enabled (it is by default). Then imgcat (https://iterm2.com/utilities/imgcat) should show images via IIP.

To repro - if you omit the size param in the sequence, images will not show up.

[hzeller]

Alright, looks like it is the raw image size. Can you do a git pull and test again ? I tested it the change with vscode terminal.

[jerch]

A quick one-liner I guess :smile_cat:

one sec...

[hzeller]

Here is an AppImage to avoid compiling from scratch.

[jerch]

Ah just got through the compilation - yupp works with the -pi switch - Thanks alot :heart: .

[hzeller]

FYI, this is part of the v1.5.3 release.

That also includes some more detection in the VSCode context, so it automatically switches to iterm graphics in vscode, and also can extract the width and height of a character cell to be able to show animations and columnized output.

[jerch]

Sidenote on size ioctls like TIOCGWINSZ:

Its indeed not safe to assume, that the OS implements the pixel notion in its tty/termios abstraction - it is not specced on POSIX side (there a ~5ys old attempt to add that). ConPty/Windows does not implement it (which is partially the reason why vscode terminal relies on those old DEC size sequences). Still all major remaining unix derivates implement the pixel stuff as far as I am aware (at least on Linux/FreeBSD/MacOS/Solaris, no clue about AIX here). Next obstacle to consider are different ssh libs and whether they transmit these ioctl datapoints (idk, have not checked it).

All in all - the size sequences are a safer bet, although a bit more cumbersome to work with on cmdline appside due the needed message cycling. For best TE+OS coverage prolly check for ioctl first and use the CSI t sequences as a fallback.

[hzeller]

Yep, the ioctl() first, then fallback query is now exactly how I do it