Fix crash with focus output and scratchpad (#6079)

The crash was brought up in a comment in
https://github.com/i3/i3/discussions/6076#discussioncomment-9536969

The cause is that the command criteria are matching a window in the
scratchpad. In that case, the assertion in get_output_for_con() fails.
That happens because there is no `Output` for the `Con` output of a
scratchpad window.

I've decided to *not* remove the offending assertion but rather rely on
the caller not using the function with internal containers. My reasoning
is:
1. If get_output_for_con can return NULL then the caller will either
segfault (which is worse) or needs to check the return for NULL.
2. The case where the return can be NULL is already known, it happens
for internal containers.
3. Therefore, the caller should already prevent the situation with a
call to con_is_internal(). Thus, the `assert`ion can remain.

There is also the potential fix of con_get_workspace returning some
arbitrary output (e.g. first in the list or currently focused one)
instead of NULL. This can lead to more tricky to catch bugs.

.gitignore

@@ -50,4 +50,3 @@ LAST_VERSION
 # it is up to you to arrange for it to be ignored by git,
 # e.g. by listing your directory in .git/info/exclude.
 /build
-

include/output.h

@@ -31,8 +31,10 @@ Output *get_output_from_string(Output *current_output, const char *output_str);
 char *output_primary_name(Output *output);
 
 /**
- * Returns the output for the given con.
- *
+ * Retrieves the output for a given container. Never returns NULL.
+ * There is an assertion that _will_ fail if the container is inside an
+ * internal workspace. Use con_is_internal() if needed before calling this
+ * function.
  */
 Output *get_output_for_con(Con *con);
 

release-notes/bugfixes/4-crash-focus-output

@@ -0,0 +1 @@
+fix crash with focus output and command criteria matching scratchpad window

src/commands.c

@@ -1802,7 +1802,20 @@ void cmd_focus_output(I3_CMD, const char *name) {
         return;
     }
 
-    Output *current_output = get_output_for_con(TAILQ_FIRST(&owindows)->con);
+    /* Command critiera need to work for focus output left|right|up|down.
+     * We need to avoid using internal workspaces with get_output_for_con, so
+     * we go through all matched windows until we find a non-internal one. If
+     * there is no match, fall back to the focused one. */
+    owindow *current;
+    Con *con = focused;
+    TAILQ_FOREACH (current, &owindows, owindows) {
+        if (!con_is_internal(con_get_workspace(current->con))) {
+            con = current->con;
+            break;
+        }
+    }
+
+    Output *current_output = get_output_for_con(con);
     Output *target_output = user_output_names_find_next(&names, current_output);
     user_output_names_free(&names);
     bool success = false;

src/output.c

@@ -54,6 +54,12 @@ char *output_primary_name(Output *output) {
     return SLIST_FIRST(&output->names_head)->name;
 }
 
+/*
+ * Retrieves the output for a given container. Never returns NULL.
+ * There is an assertion that _will_ fail if the container is inside an
+ * internal workspace. Use con_is_internal() if needed before calling this
+ * function.
+ */
 Output *get_output_for_con(Con *con) {
     Con *output_con = con_get_output(con);
     Output *output = get_output_by_name(output_con->name, true);

testcases/t/321-crash-criteria-scratchpad.t

@@ -0,0 +1,46 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • https://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • https://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • https://build.i3wm.org/docs/ipc.html
+#   (or docs/ipc)
+#
+# • https://i3wm.org/downloads/modern_perl_a4.pdf
+#   (unless you are already familiar with Perl)
+#
+# Verify that i3 does not crash when command criteria that match a scratchpad
+# window are used with the focus output command or other commands
+# Ticket: #6076
+# Bug still in: 4.23-43-g822477cb
+use i3test;
+
+my $ws = fresh_workspace;
+my $win = open_window;
+cmd "move scratchpad";
+
+sub cmd_on_w {
+    local $Test::Builder::Level = $Test::Builder::Level + 1;
+    my $c = shift;
+    subtest "$c" => sub {
+        my $result = cmd '[id="' . $win->id . '"] ' . $c;
+        is($result->[0]->{success}, 1, "command succeeded");
+        is(@{get_ws($ws)->{floating_nodes}}, 0, 'no floating windows on workspace');
+        is(@{get_ws($ws)->{nodes}}, 0, 'no nodes on workspace');
+    }
+}
+
+cmd_on_w 'nop';
+cmd_on_w 'focus output left';
+cmd_on_w 'focus left';
+cmd_on_w 'floating disable';
+cmd_on_w 'floating enable';
+
+does_i3_live;
+
+done_testing;