journalctl: don't skip over messages not matching the cursor

When --after-cursor=/--cursor-file= is used together with a journal
filter, we still skipped over the first matching entry even if it wasn't
the entry the cursor points at, thus missing one "valid" entry
completely. Let's fix this by checking if the entry cursor after seeking
matches the user provided cursor, and skip to the next entry only when
the cursors match.

Resolves: #30288

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index e79acc65bcb501a5738f0c378f92b5d5968cd025..de1534befd2c894db37d0fd1a39852e997d242eb 100644 (file)
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -2164,10 +2164,12 @@ static int setup_event(Context *c, int fd, sd_event **ret) {
 }
 
 static int run(int argc, char *argv[]) {
-        bool need_seek = false, since_seeked = false, use_cursor = false, after_cursor = false;
+        bool need_seek = false, since_seeked = false, after_cursor = false;
         _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL;
         _cleanup_(umount_and_freep) char *mounted_dir = NULL;
         _cleanup_(sd_journal_closep) sd_journal *j = NULL;
+        _cleanup_free_ char *cursor_from_file = NULL;
+        const char *cursor = NULL;
         int n_shown, r, poll_fd = -EBADF;
 
         setlocale(LC_ALL, "");
@@ -2444,8 +2446,7 @@ static int run(int argc, char *argv[]) {
         }
 
         if (arg_cursor || arg_after_cursor || arg_cursor_file) {
-                _cleanup_free_ char *cursor_from_file = NULL;
-                const char *cursor = arg_cursor ?: arg_after_cursor;
+                cursor = arg_cursor ?: arg_after_cursor;
 
                 if (arg_cursor_file) {
                         r = read_one_line_file(arg_cursor_file, &cursor_from_file);
@@ -2458,30 +2459,40 @@ static int run(int argc, char *argv[]) {
                         }
                 } else
                         after_cursor = arg_after_cursor;
+        }
 
-                if (cursor) {
-                        r = sd_journal_seek_cursor(j, cursor);
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to seek to cursor: %m");
+        if (cursor) {
+                r = sd_journal_seek_cursor(j, cursor);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to seek to cursor: %m");
 
-                        use_cursor = true;
+                r = sd_journal_step_one(j, !arg_reverse);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to iterate through journal: %m");
+
+                if (after_cursor && r > 0) {
+                        /* With --after-cursor=/--cursor-file= we want to skip the first entry only if it's
+                         * the entry the cursor is pointing at, otherwise, if some journal filters are used,
+                         * we might skip the first entry of the filter match, which leads to unexpectedly
+                         * missing journal entries. */
+                        int k;
+
+                        k = sd_journal_test_cursor(j, cursor);
+                        if (k < 0)
+                                return log_error_errno(k, "Failed to test cursor against current entry: %m");
+                        if (k > 0)
+                                /* Current entry matches the one our cursor is pointing at, so let's try
+                                 * to advance the next entry. */
+                                r = sd_journal_step_one(j, !arg_reverse);
                 }
-        }
-
-        if (use_cursor) {
-                if (!arg_reverse)
-                        r = sd_journal_next_skip(j, 1 + after_cursor);
-                else
-                        r = sd_journal_previous_skip(j, 1 + after_cursor);
 
-                if (after_cursor && r < 2) {
+                if (r == 0) {
                         /* We couldn't find the next entry after the cursor. */
                         if (arg_follow)
                                 need_seek = true;
                         else
                                 arg_lines = 0;
                 }
-
         } else if (arg_until_set && (arg_reverse || arg_lines_needs_seek_end())) {
                 /* If both --until and any of --reverse and --lines=N is specified, things get
                  * a little tricky. We seek to the place of --until first. If only --reverse or

diff --git a/test/units/testsuite-04.journal.sh b/test/units/testsuite-04.journal.sh
index 3efac616e086e7a711cee7272d17c6fef1fa09fa..4d9e48717acd6847ecc212ce9fb9961b7245abad 100755 (executable)
--- a/test/units/testsuite-04.journal.sh
+++ b/test/units/testsuite-04.journal.sh
@@ -241,3 +241,27 @@ diff -u /tmp/lb1 - <<'EOF'
 [{"index":-3,"boot_id":"5ea5fc4f82a14186b5332a788ef9435e","first_entry":1666569600994371,"last_entry":1666584266223608},{"index":-2,"boot_id":"bea6864f21ad4c9594c04a99d89948b0","first_entry":1666569601005945,"last_entry":1666584347230411},{"index":-1,"boot_id":"4c708e1fd0744336be16f3931aa861fb","first_entry":1666569601017222,"last_entry":1666584354649355},{"index":0,"boot_id":"35e8501129134edd9df5267c49f744a4","first_entry":1666569601009823,"last_entry":1666584438086856}]
 EOF
 rm -rf "$JOURNAL_DIR" /tmp/lb1
+
+# Check that using --after-cursor/--cursor-file= together with journal filters doesn't
+# skip over entries matched by the filter
+# See: https://github.com/systemd/systemd/issues/30288
+UNIT_NAME="test-cursor-$RANDOM.service"
+CURSOR_FILE="$(mktemp)"
+# Generate some messages we can match against
+journalctl --cursor-file="$CURSOR_FILE" -n1
+systemd-run --unit="$UNIT_NAME" --wait --service-type=exec bash -xec "echo hello; echo world"
+journalctl --sync
+# --after-cursor= + --unit=
+# The format of the "Starting ..." message depends on StatusUnitFormat=, so match only the beginning
+# which should be enough in this case
+[[ "$(journalctl -n 1 -p info -o cat --unit="$UNIT_NAME" --after-cursor="$(<"$CURSOR_FILE")" _PID=1 )" =~ ^Starting\  ]]
+# There should be no such messages before the cursor
+[[ -z "$(journalctl -n 1 -p info -o cat --unit="$UNIT_NAME" --after-cursor="$(<"$CURSOR_FILE")" --reverse)" ]]
+# --cursor-file= + a journal filter
+diff <(journalctl --cursor-file="$CURSOR_FILE" -p info -o cat _SYSTEMD_UNIT="$UNIT_NAME") - <<EOF
++ echo hello
+hello
++ echo world
+world
+EOF
+rm -f "$CURSOR_FILE"