bugGNU Screen - Bugs: bug #52667, Off-by-one in RGB truecolor escape...

 
 

bug #52667: Off-by-one in RGB truecolor escape sequence parameter indices

Submitter:  Egmont Koblinger <egmont>
Submitted:  Thu 14 Dec 2017 08:45:06 PM UTC
   
 
Category:  Program Logic Severity:  3 - Normal
Priority:  * 5 - Normal Status:  None
Privacy:  Public Assigned to:  None
Open/Closed:  Open Release:  Cur Dev Sources
Fixed Release:  None Planned Release:  None
Work Required:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 24 Feb 2022 06:52:54 PM UTC, comment #8: 

I have also made V4-patches for the two relevant pieces: the terminfo and ignoring unused grouped arguments.

(file #52927, file #52928)

Carl Drougge <drougge>
Thu 24 Feb 2022 06:50:43 PM UTC, comment #7: 

I have prepared three patches for V5 for this:

1. Ignore unused extra arguments in :-separated group.
2. Allow (ignored) color_space_id if group is long enough.
3. Update terminfo to use : in 38 and 48 sequences.

(file #52924, file #52925, file #52926)

Carl Drougge <drougge>
Sun 18 Nov 2018 03:42:04 PM UTC, comment #6: 

Replying to comment 3: workaround for missing color was applied to master.

Amadeusz Sławiński <amade>
Group administrator
Tue 21 Aug 2018 07:28:27 PM UTC, comment #5: 

I was wondering if there was already a follow up on this issue?

Anonymous
Tue 19 Jun 2018 05:37:56 AM UTC, comment #4: 

Yes, I know about this, something was changed in ncurses and it seems that tparm ignores 3rd and further parameters, should probably look if there is some workaround or talk to ncurses guy if its intentional.

Amadeusz Sławiński <amade>
Group administrator
Tue 19 Jun 2018 03:54:36 AM UTC, comment #3: 

I compile screen from source to get truecolor support. When I upgraded ubuntu recently, truecolor support broke. Specifically, I am seeing the wrong colors being output, but not a downgrade to 256 colors or similar. Is this bug what I should be following to see when this will be fixed or is this a separate issue?

To see what I mean by wrong colors, see https://stackoverflow.com/questions/50826467/screen-truecolor-ubuntu-18-04-broke

Anonymous
Sat 16 Dec 2017 02:38:29 PM UTC, comment #2: 

The problem is: in practice, everyone seems to be using the semicolon format (without color space id). Removing support for this would break the look of currently existing truecolor apps until one day they switch to the correct colon separated (+ color space id) format. Which requires all the terminal emulators to catch up first, then wait for a couple of years so that most users have a new enough terminal emulator.

I'd love to see the whole world fixing it, but if it happens eventually at all (which I cannot guarantee) then it'll take years. And while it's great to have players who "push" the world towards the fix (by starting to reject the old format early on), I'm afraid it might be too early to start that and would cause some users turning away from using screen and/or truecolors.

In case of screen, of course it's two different questions in one: Which formats does it accept as input from apps, and which format does it use towards the host terminal emulator? My proposal: For now, it should accept at least two formats (the popular and the correct one) as input, and emit the popular one towards the host emulator (or have a config option, defaulting to this). Seeing how terminal emulators catch up with the fix, in a couple of years the sequences sent to the host emulator might be flipped to the correct one, and probably not right then but yet another couple of years later support for the old (currently popular) mode could be dropped.

It might also happen that there's not enough motivation in the terminal emulator world to fix it, and we get stuck with the currently frequently used semicolon format forever, I cannot foresee it.

Egmont Koblinger <egmont>
Sat 16 Dec 2017 02:00:41 PM UTC, comment #1: 

So...

I think the correct thing would be to just remove the ambiguity and just support correct escape codes.

Amadeusz Sławiński <amade>
Group administrator
Thu 14 Dec 2017 08:45:06 PM UTC, original submission:  

screen accepts 256-color escape sequences like CSI 38;5;INDEXm and truecolor ones like CSI 38;2;R;G;Bm (also with colon as delimiter instead of semicolon). (And similarly 48 for background.)

In fact, screen seems to treat colon and semicolon identically everywhere, e.g. CSI 4:3m switches to underlined(4) and italic(3), just as CSI 4;3m does.

GNOME Terminal developer Christian has recently discovered that pretty much everyone (including xterm, VTE (gnome-terminal), screen etc.) misread/misimplemented the ITU Recommendation T.416 at truecolor escape sequences (and/or copied the false interpretation from each other).

Carefully reading the spec (which only allows ':' as the separator) the actual format is:

  CSI 38:5:INDEXm

and

  CSI 38:2:[color_space_id]:R:G:B[:unused:tolerance:color_space_for_tolerance]m

That is, at truecolor, there's an extra parameter at the beginning before the three color components, plus optional parameters at the end. For example, to get red=255, green=128, blue=64 (an orange-ish color), the sequence could be

  CSI 38:2::255:128:64m

Most apps that support truecolor emit sequences like CSI 38;2;R;G;Bm, because not too many terminal emulators support ':' as the delimiter, and understandably app developers take the easy path which works everywhere. So I wouldn't touch the parameters of the semicolon-based sequence, it'd break existing truecolor support at many places, and there's no way to inject that missing parameter without breaking compatibility with existing (faulty) implementations. I think we should say this format has become the de facto standard.

With the colon as separator, however, it would be nice to fix it and adhere to the de jure standard. That is, take another parameter before the red channel, plus allow optional ones after blue.

For backwards compatibility with the colon-based format and the color_space_id omitted, you may want to still accept only 3 additional parameters after "38:2:" and treat them as R, G and B. But as soon as 4 or more parameters follow "38:2:", the first one should be the color_space_id, then the three color channels, then perhaps further additional ones.

The end of the optional parameters is obviously denoted either by the trailing 'm', or the semicolon ';' which allows further independent SGR attributes to follow.

See https://gist.github.com/XVilka/8346728#gistcomment-2008553 for my opinion about why colon (as stated by the documentation) is a good choice for subparameters and semicolon (in widespread use) is a bad one; and why terminal emulators (such as screen) shouldn't treat them equally.

For more details about this particular issue, please see

https://bugzilla.gnome.org/show_bug.cgi?id=791456

https://gist.github.com/XVilka/8346728#gistcomment-2285644

Egmont Koblinger <egmont>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by drougge (Updated the item)
  • -email is unavailable- added by amade (Posted a comment)
  • -email is unavailable- added by egmont (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-02-24 drougge Attached File- Added 0001-Update-terminfo-to-use-instead-of-in-38-and-48.v4.patch, #52927
        Attached File- Added 0002-Ignore-unused-extra-arguments-to-SGR-values.v4.patch, #52928
    2022-02-24 drougge Attached File- Added 0001-Ignore-unused-extra-arguments-to-SGR-values.v5.patch, #52924
        Attached File- Added 0002-Allow-color_space_id-in-truecolor-sequence-using.v5.patch, #52925
        Attached File- Added 0003-Update-terminfo-to-use-instead-of-in-38-and-48.v5.patch, #52926

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code