From 0230545fea019eef5c01ca9f38f5a235323931be Mon Sep 17 00:00:00 2001 From: OrtegaMatias <212189024+OrtegaMatias@users.noreply.github.com> Date: Mon, 20 Jul 2026 10:18:56 -0400 Subject: [PATCH] Fix container logs -n truncating the oldest line past a read chunk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `container logs -n N` read the log file backwards in 1024-byte chunks and stopped at `lines.count < n`. When the last N lines spanned more than one chunk, the oldest line in the buffer was a fragment truncated at the chunk boundary and was returned as-is — e.g. a single log line longer than 1024 bytes with `-n 1` returned only its last 1024 bytes. Extract the backward-read into a testable `lastLines(fh:n:)` and read one line past N (`<= n`) so the partial leading line is dropped by `suffix(n)`, or stop at the start of the file. The full-log path is unchanged. --- .../Container/ContainerLogs.swift | 55 ++++++++------ .../ContainerLogsTailTests.swift | 75 +++++++++++++++++++ 2 files changed, 109 insertions(+), 21 deletions(-) create mode 100644 Tests/ContainerCommandsTests/ContainerLogsTailTests.swift diff --git a/Sources/ContainerCommands/Container/ContainerLogs.swift b/Sources/ContainerCommands/Container/ContainerLogs.swift index e0eb3511e..2b4247093 100644 --- a/Sources/ContainerCommands/Container/ContainerLogs.swift +++ b/Sources/ContainerCommands/Container/ContainerLogs.swift @@ -63,27 +63,7 @@ extension Application { follow: Bool ) async throws { if let n { - var buffer = Data() - let size = try fh.seekToEnd() - var offset = size - var lines: [String] = [] - - while offset > 0, lines.count < n { - let readSize = min(1024, offset) - offset -= readSize - try fh.seek(toOffset: offset) - - let data = fh.readData(ofLength: Int(readSize)) - buffer.insert(contentsOf: data, at: 0) - - if let chunk = String(data: buffer, encoding: .utf8) { - lines = chunk.components(separatedBy: .newlines) - lines = lines.filter { !$0.isEmpty } - } - } - - lines = Array(lines.suffix(n)) - for line in lines { + for line in try Self.lastLines(fh: fh, n: n) { print(line) } } else { @@ -109,6 +89,39 @@ extension Application { } } + /// Returns the last `n` complete lines from `fh`, reading backwards in + /// 1024-byte chunks. Exposed (internal) for testing. + /// + /// The loop keeps reading until it has accumulated *more* than `n` + /// non-empty lines, or it reaches the start of the file. Reading one + /// line past `n` guarantees the oldest returned line is complete: the + /// first line in the buffer may be a fragment truncated at a chunk + /// boundary, and `suffix(n)` drops it. Stopping at exactly `n` + /// (`lines.count < n`) could return that fragment as the oldest line + /// whenever the last `n` lines span more than one 1024-byte chunk. + static func lastLines(fh: FileHandle, n: Int) throws -> [String] { + var buffer = Data() + let size = try fh.seekToEnd() + var offset = size + var lines: [String] = [] + + while offset > 0, lines.count <= n { + let readSize = min(1024, offset) + offset -= readSize + try fh.seek(toOffset: offset) + + let data = fh.readData(ofLength: Int(readSize)) + buffer.insert(contentsOf: data, at: 0) + + if let chunk = String(data: buffer, encoding: .utf8) { + lines = chunk.components(separatedBy: .newlines) + lines = lines.filter { !$0.isEmpty } + } + } + + return Array(lines.suffix(n)) + } + private static func followFile(fh: FileHandle) async throws { _ = try fh.seekToEnd() let stream = AsyncStream { cont in diff --git a/Tests/ContainerCommandsTests/ContainerLogsTailTests.swift b/Tests/ContainerCommandsTests/ContainerLogsTailTests.swift new file mode 100644 index 000000000..cfac4283f --- /dev/null +++ b/Tests/ContainerCommandsTests/ContainerLogsTailTests.swift @@ -0,0 +1,75 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Foundation +import Testing + +@testable import ContainerCommands + +struct ContainerLogsTailTests { + private func withTempLog(_ contents: String, _ body: (FileHandle) throws -> Void) throws { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("container-logs-tail-\(UUID().uuidString).log") + try Data(contents.utf8).write(to: url) + let fh = try FileHandle(forReadingFrom: url) + defer { + try? fh.close() + try? FileManager.default.removeItem(at: url) + } + try body(fh) + } + + @Test("returns the last n complete lines for short input") + func lastNShortLines() throws { + let contents = (1...10).map { "line-\($0)" }.joined(separator: "\n") + "\n" + try withTempLog(contents) { fh in + let lines = try Application.ContainerLogs.lastLines(fh: fh, n: 3) + #expect(lines == ["line-8", "line-9", "line-10"]) + } + } + + @Test("n larger than the line count returns every line") + func nLargerThanLineCount() throws { + try withTempLog("a\nb\n") { fh in + let lines = try Application.ContainerLogs.lastLines(fh: fh, n: 10) + #expect(lines == ["a", "b"]) + } + } + + @Test("a single line longer than the 1024-byte chunk is not truncated") + func longSingleLineNotTruncated() throws { + // Regression: a log line larger than the 1024-byte backward-read chunk + // was returned with its beginning cut off by `container logs -n 1`. + let long = "START" + String(repeating: "X", count: 3000) + "END" + try withTempLog(long + "\n") { fh in + let lines = try Application.ContainerLogs.lastLines(fh: fh, n: 1) + #expect(lines == [long]) + } + } + + @Test("multiple lines spanning several chunks keep the oldest line intact") + func longLinesSpanningChunks() throws { + // Each line is 800 bytes; the last two together exceed one 1024-byte + // chunk, which previously truncated the older of the two. + let a = String(repeating: "A", count: 800) + let b = String(repeating: "B", count: 800) + let c = String(repeating: "C", count: 800) + try withTempLog([a, b, c].joined(separator: "\n") + "\n") { fh in + let lines = try Application.ContainerLogs.lastLines(fh: fh, n: 2) + #expect(lines == [b, c]) + } + } +}