Skip to content

bodyToKeys produces a phantom empty key for empty bodies and blank lines #32

Description

@dolph

Summary

bodyToKeys in httpclient.go:117 does strings.Split(strings.TrimSuffix(body, "\n"), "\n"). For an empty response body (or a body of just "\n") that returns [""] — a slice with one empty string — which gets logged as "Found 1 key(s)" and emitted to stdout as a blank line.

Reproduction

bodyToKeys([]byte(""))         // → []string{""}, logs "Found 1 key(s)"
bodyToKeys([]byte("\n"))       // → []string{""}, logs "Found 1 key(s)"
bodyToKeys([]byte("a\n\nb\n")) // → []string{"a", "", "b"}, logs "Found 3 key(s)"

Why this matters

Mostly cosmetic. sshd happily skips empty lines, so the end-to-end behavior is fine. Two real (but small) consequences:

  1. The "Found N key(s)" log message overstates the count by one for any source that returned no keys — operators reading the log to triage "why did Alice get denied?" see a misleading "Found 1 key(s)" for a source that actually returned nothing.
  2. Blank lines mid-response inflate the count similarly.

This isn't security-relevant on its own, but it's the kind of bug that makes log analysis subtly misleading on a tool whose log is part of the audit trail.

Approach

Filter empty entries out after splitting, and adjust the count:

func bodyToKeys(body []byte) []string {
    keys := make([]string, 0)
    for _, line := range strings.Split(strings.TrimSpace(string(body)), "\n") {
        line = strings.TrimSpace(line)
        if line == "" {
            continue
        }
        keys = append(keys, line)
    }
    log.Printf("Found %d key(s)", len(keys))
    return keys
}

strings.TrimSpace (instead of just TrimSuffix(s, "\n")) handles trailing CRLF and stray whitespace too. Per-line TrimSpace defends against trailing-CR from CRLF-terminated upstreams.

Acceptance criteria

  • bodyToKeys([]byte("")) returns []string{} (length 0), not []string{""}.
  • bodyToKeys([]byte("\n")) returns []string{}.
  • bodyToKeys([]byte("a\n\nb\n")) returns []string{"a", "b"}, length 2.
  • Log line shows the correct key count for each of the above.
  • Existing tests still pass.

Files

  • /home/user/ussher/httpclient.go
  • /home/user/ussher/httpclient_test.go (small new table-driven test)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    LowbugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions