| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
|
|
The manager mocks lived in production source so that internal/mobius
tests could import them (test files are not importable across
packages), which pulled testify into the production dependency graph
of hotline importers, counted the mocks against coverage, and put them
in the library's godoc. They now live in hotline/hltest, an
httptest-style test-support package.
The hotline package's own in-package tests cannot import hltest (import
cycle), so the mocks they use are duplicated in mocks_test.go;
conformance assertions in both files catch signature drift.
MockAccountManager was only ever used inside internal/mobius and moves
to a _test.go file there.
|
|
Add a protocol-level end-to-end suite (in-process fully wired server on
an ephemeral port pair, driven by hotline.Client over TCP) covering
handshake, login, public and private chat, message board, threaded
news, file list/download/upload, account admin, disconnect
notification, and shutdown broadcast. The harness retries on a fresh
port pair when another process steals a probed port before
ListenAndServe binds it, and Server gains WithConnectionRateLimit so
tests can disable the per-IP connection throttle.
Add native fuzz tests for Transaction, Field, and flattened file
object decoding, and fix the bugs the new tests surfaced:
- Transaction.Write panicked on out-of-range attacker-controlled size
fields, and transactionScanner's uint32 length addition could wrap
and yield a truncated token. The information fork size declared in
an untrusted fork header is now bounded too.
- Client keepalive read c.done unsynchronized while Disconnect
replaces it under the mutex.
- The shared Agreement's Seek+ReadAll login path raced concurrent
logins; the server now prefers an AgreementBytes() snapshot.
Fill unit-test gaps (main's config-copy helpers, file resume data,
ReloaderFunc, R2 error injection and env validation) and add a CI test
workflow (build/vet + race-enabled shuffled suite), fixed lint
workflow triggers with golangci-lint v2.6, and Makefile test/cover/
lint/fuzz targets.
|
|
NewClientConn added the connection to the ClientManager before Account
and Version were assigned, so any goroutine iterating ClientMgr.List()
during the login handshake could dereference a nil Account (e.g.
HandleSetUser reading c.Account.Login) and panic. Account was also read
and written across goroutines with no synchronization: HandleSetUser
wrote c.Account.Access on another client's connection while that
client's own transaction loop read it in Authorize.
The connection is now added to the manager in handleNewConnection only
once Account, Version, UserName, and Flags are initialized, so a
published client is always fully formed. Failed logins never publish
the connection at all; Disconnect's manager delete is a no-op for them.
Account joins the mutex-guarded session state with accessors in the
established style: SetAccount/GetAccount, SetAccountAccess for the one
post-login mutation, AccessBytes for building transaction fields, and
Authorize now takes the read lock. All cross-goroutine call sites go
through the accessors.
HandleSetUser previously recomputed the admin flag from the client's
stale access level and only converged on the following edit; the access
update now happens before the recompute.
|
|
LoadConfig unconditionally rewrote a relative FileRoot to an absolute
path under the config dir. R2FileStore.key() then embedded that local
path in every object key (e.g. Users/jhalter/.../config/Files/...), so
bucket contents were tied to the host's directory layout and orphaned
by moving the config dir or pointing another server at the bucket.
FileRoot is a path within the selected file store's namespace, so only
the backend selection in main knows how to resolve it: the os backend
resolves relative values against the config dir as before, while the
memory and r2 backends keep the configured value verbatim, yielding
portable keys like Files/foo.txt. An absolute FileRoot combined with an
object-store backend now logs a warning.
WithConfig copies the config struct, so the option is appended after
the backend selection mutates FileRoot rather than before.
|
|
The FileStore interface returned concrete *os.File from Open/Create/
OpenFile, which no non-filesystem backend (e.g. S3/R2) can produce, and
many file-library hot paths bypassed the interface entirely with direct
os.* / filepath.Walk calls.
Widen the interface to return io.ReadCloser / io.WriteCloser and add
ReadDir, ReadLink, and Walk so directory traversal no longer escapes the
abstraction. Route every file-library call site (fork writers, upload/
download handlers, GetFileNameList, CalcTotalSize/CalcItemCount, the set-
file-info folder rename) through the injected FileStore, and add a
WithFileStore option. OSFileStore keeps byte-identical behavior.
DownloadHandler now nil-guards the optional resource-fork reader instead
of relying on *os.File's nil-receiver tolerance, so a backend returning an
untyped-nil reader does not panic.
|
|
Replace the leaky *redis.Client field on Server with a PresenceTracker
interface that receives connect/rename/disconnect lifecycle events. This
removes Redis-specific set encoding from hotline/server.go and the session
handlers, and drops the redis dependency from the hotline package.
The Redis implementation moves to mobius.RedisPresenceTracker, which owns
the legacy "login::ip"/"login:nickname:ip" set encoding so existing
deployments keep working. The API server reads online users through a new
OnlineLister interface and falls back to the in-memory ClientMgr when no
presence tracker is configured. Startup clearing of stale online state now
happens unconditionally when Redis is configured, not only when the API
server is enabled.
|
|
main.go's reloadFunc reached through the server's interface fields with
concrete type assertions (srv.MessageBoard.(*mobius.FlatNews), etc.) to
trigger SIGHUP/API reloads, leaking storage implementation details past
the manager interfaces.
Storage backends now implement a one-method Reloader interface, with
compile-time assertions for FlatNews, BanFile, ThreadedNewsYAML, and
Agreement. BanFile.Load and ThreadedNewsYAML.Load are renamed Reload
for a uniform method set, matching FlatNews's existing convention of
using Reload for the initial load as well.
main.go registers each backend in a named reloader list as it is
constructed, and reloadFunc iterates the list. The banner reload is a
ReloaderFunc that also performs the initial load, and the Redis-backed
ban list simply registers no reloader, replacing the old type-switch
special case.
|
|
ClientConn's mutable session state (Flags, UserName, Icon, IdleTime,
AutoReply) was guarded inconsistently: two mutexes (FlagsMU and mu)
covered some paths while others mutated or read the fields with no
locking at all, including HandleSetClientUserInfo, HandleUpdateUser
(which writes other clients' admin flag), the login flow, the HTTP API
handlers, and the keepalive loop. Consolidate on a single mutex with
accessor methods (SetFlag/IsFlagSet/FlagBytes, SetUserName/GetUserName,
and so on) used by all production code; direct field access remains for
test construction. The idle/away logic moves into incrementIdleTime
and clearIdleAndAway helpers that report whether a notification is
needed, so SendAll is no longer called while holding the lock.
HandleRejectChatInvite also no longer appends to the username slice,
which could write past its length into the backing buffer.
The server banner is now behind Banner/SetBanner with an RWMutex: the
SIGHUP reload previously reassigned the field while banner download
goroutines read it, and nilled it when the file read failed. Reload
now keeps the previous banner on failure.
Per-IP rate limiter entries now record a last-seen time, and the
keepalive ticker evicts entries idle for over seven days, so the map
no longer grows unboundedly with each unique client IP.
|
|
The outbox channel spawned one goroutine per outbound transaction, so
concurrent sends to the same client could interleave bytes within the
transaction framing, per-client message ordering was not guaranteed,
and a slow client accumulated unbounded goroutines.
Each ClientConn now has a bounded send queue drained by a single writer
goroutine, which serializes writes and preserves enqueue order. Send
never blocks: if a client's queue overflows, its connection is closed
and the read loop performs the usual disconnect cleanup. Server.Send
routes transactions to the target client's queue, replacing
processOutbox and sendTransaction. Handler signatures are unchanged.
Disconnect now removes the client from the manager before notifying
peers so no new transactions are routed to a departing client, then
idempotently closes its send queue.
New tests cover write ordering, framing integrity under concurrent
senders, the slow-client disconnect policy, and a Send/Disconnect race
exercise (run with -race).
|
|
Break up the 2,349-line transaction_handlers.go and its 6,716-line test
file into per-domain files (chat, files, transfers, accounts, news,
session), moving code verbatim with no signature or behavior changes.
Error message constants move to errors.go and shared test fixtures to
helpers_test.go; transaction_handlers.go retains only RegisterHandlers.
|
|
Startup log:
- Add interface, port, and fileTransferPort fields to the "Hotline
server started" line so operators can see what the server bound to.
- Resolve an empty -interface flag to 0.0.0.0 for display.
Levels:
- Demote the two Redis startup messages (ban management, cleared online
users) from Info to Debug.
Consistency:
- Standardize the error field key to "err" (was "Err" in a few account
handlers) and lowercase "Account" -> "account".
- Standardize the remote-address key to "remoteAddr" (was "RemoteAddr").
- Replace fmt.Sprintf in the tracker-registration message and string
concatenation in the config-dir-init and ban-disconnect messages with
structured fields; use the standard "err" key.
- Give the two bare rLogger.Error(err.Error()) file-transfer calls a
descriptive message and an "err" field.
logger.go:
- Only attach the rotating lumberjack file writer when --log-file is set.
An empty Filename made lumberjack write to a temp file by default.
|
|
A malicious or buggy client could send transaction fields with the wrong
length and trigger runtime panics (slice/index out of range, slice-to-array
conversion, nil deref) in the parsing and handler code. These were caught by
the connection-level recover, so they dropped the client connection and dumped
a stack trace to stdout rather than crashing the process, but they are still
incorrect behavior, a log-flood vector, and a latent crash if the recover
scope ever changes.
Fix at the source and harden the safety net:
- Add ClientIDFromBytes / ChatIDFromBytes helpers that return ok=false on a
length mismatch, and use them in the transaction handlers instead of direct
[2]byte(...) / [4]byte(...) conversions on field data. Nil-check ClientMgr.Get
results, and length-guard FieldOptions and the HandleUpdateUser sub-field
header. Malformed input now yields an error reply (or a clean no-op for
reply-less handlers) instead of panicking.
- Bounds-check FileResumeData.UnmarshalBinary (header length and fork count)
and guard the ForkInfoList[0] accesses against an empty list.
- Bounds-check FlatFileInformationFork parsing (reachable on upload): validate
the fixed header, name, and comment lengths, and fix a latent 72+nameSize
uint16 overflow. Route Write through UnmarshalBinary.
- panic.go: stop printing stack traces to stdout (keep structured logging) so a
client cannot flood stdout by repeatedly triggering a panic.
- handleTransaction: recover per-transaction so one malformed request no longer
tears down the whole client connection.
Adds tests for the new helpers, the hardened resume-data and flat-file-object
decoders, and handler-level malformed-ID handling.
|
|
Many handlers in internal/mobius/transaction_handlers.go returned an empty
transaction slice on error (return res / return nil), so the client received
no reply and the operation silently no-opped, with no log in most cases.
Convert these silent discards across the file to a consistent convention:
log the underlying cause via cc.Logger.Error and return cc.NewErrReply with a
user-facing message. Covers the file-operation, file-transfer, news,
user/account, and chat handler groups. Genuinely intentional no-reply paths
(target user offline, banned-nickname disconnect) are kept silent but now
carry an explanatory comment.
Also fixes adjacent defects found along the way:
- HandleSetFileInfo: a non-IsNotExist error from os.Rename during a directory
rename was swallowed; it now returns an error reply.
- HandleUploadFile: when a resume is requested but no .incomplete file exists,
fall back to a normal upload reply instead of discarding the reply.
- hotline.DecodeNewsPath: previously panicked on a 1-byte client-supplied
field (slice out of range) and silently produced empty path components on
truncated input. It now validates length and framing and returns an error,
which the handlers already surface.
Adds regression tests for the new error replies, the upload resume fallback,
and DecodeNewsPath's malformed-input handling.
|
|
Replace the map-backed Stats counter with a fixed [numStats]int array
indexed by a new StatKey enum, eliminating the hand-maintained map
initialization and the parallel string-keyed Values() map.
Fix a check-then-act race in the connection peak tracking: the old
Get-then-Set across two lock acquisitions could let concurrent
connections clobber each other's update. The new atomic Max method does
the compare-and-set under a single lock.
Values() now returns a typed StatValues struct whose JSON tags preserve
the existing /api/v1/stats wire format.
|
|
Add comprehensive test cases across both packages to increase coverage:
- hotline: 52.4% → ~55% (Disconnect, handleTransaction, SendAll,
sendBanMessage, MemClientMgr, and other tests)
- internal/mobius: 75.9% → ~80% (HandleUpdateUser, HandleDeleteUser,
HandleSetUser, HandleNewUser, HandleUserBroadcast success paths)
|
|
13 types implemented identical offset-based io.Reader patterns with
5-7 lines of copy-and-offset-tracking code each. Extract a shared
readFrom(p, offset, data) helper and reduce each Read() method to a
one-liner delegating to it.
|
|
Replace hardcoded Mac Roman encoding globals with a configurable Encoding
field in config.yaml. Servers that exclusively serve modern UTF-8 clients
can now set Encoding: utf8 to disable Mac Roman conversion. The default
remains "macintosh" for backward compatibility.
- Add Encoding field to Config struct (macintosh|utf8)
- Store TextDecoder/TextEncoder on Server, initialized from config
- Add TextDecoder()/TextEncoder() accessors on ClientConn
- Pass decoder/encoder explicitly to ReadPath and GetFileNameList
- Remove package-level txtEncoder/txtDecoder globals
- Add warning log when files are skipped due to encoding errors
- Log and return error replies in HandleGetFileNameList on failure
- Document the new config option in docs/text-encoding.md
|
|
ReadPath() and HandleNewFolder decoded the entire joined path from Mac
Roman, including the fileRoot prefix which is already UTF-8. This caused
file listing failures when config paths or FileRoot values contained
non-ASCII characters. Now only client-provided path components (subPath,
fileName/folderName) are decoded before joining with fileRoot.
|
|
Return errors to clients on write failures instead of silently succeeding.
Add rollback logic to BanFile and ThreadedNewsYAML mutations so in-memory
state is restored when persistence fails. Extract error message constants
and add comprehensive tests for error paths and rollback behavior.
|
|
Extract ban logic into a BanMgr interface with two implementations:
- BanFile: file-based YAML storage with support for IP, username, and
nickname bans (backwards-compatible with legacy format)
- RedisBanMgr: Redis-backed implementation with permanent and temporary
ban support, using fail-safe deny-on-error behavior
This replaces scattered Redis calls in API handlers, transaction
handlers, and server connection logic with unified interface calls,
removing the Redis dependency from the API server constructor and
enabling ban functionality for both file-only and Redis deployments.
|
|
|
|
- Add logging for unhandled transaction types in Client.HandleTransaction
- Fix race condition in Client.Disconnect by protecting done channel with mutex
- Add TranServerMsg to transaction type names map
- Use Time type instead of raw byte array in File.flattenedFileObject
- Improve error message in handleFileTransfer to include reference number
- Simplify return statement in HandleGetFileInfo
|
|
|
|
|
|
|
|
- Introduce Logger interface to replace direct slog dependency in Client
- Implement Write methods for NewsArtListData and NewsCategoryListData15 with support for partial/chunked writes
- Enhance transaction logging with human-readable type names
|
|
|
|
Adds new constants for file format identifiers (FormatFILP), fork types
(ForkTypeINFO), and platform identifiers (PlatformAMAC, PlatformMWIN) to
improve code maintainability and readability. Changes FlatFileForkHeader.ForkType
field from [4]byte to ForkType type alias for better compile-time type checking,
matching the pattern used in ForkInfoList.Fork.
|
|
- Add custom validator for banner file extensions (.jpg, .jpeg, .gif)
- Update HandleTranAgreed to dynamically detect banner type from file extension
- Export FileTypeFromFilename function for use across packages
- Add comprehensive test coverage for banner validation and type detection
- Improve error messages to clearly communicate allowed file extensions
|
|
Fix two failing test cases in TestHandleSetClientUserInfo:
- Corrected auto-reply test to use UserOptAutoResponse (bit 2) instead of UserOptRefusePM (bit 0)
- Updated refuse private messages test to expect correct flag value when UserOptRefuseChat sets UserFlagRefusePChat
The HandleSetClientUserInfo function was working correctly - the test expectations were wrong about how user options map to user flags.
|
|
|
|
Replace custom extractIP function and inconsistent string.Split usage with Go's standard net.SplitHostPort to ensure proper IPv6 compatibility and consistent IP address handling across Redis operations.
|
|
- Add OpenAPI specification (api.yaml) with complete endpoint documentation
- Update README with comprehensive API section including authentication and examples
- Add godoc comments to all API handlers and types for better code documentation
|
|
Updated all file close operations to use anonymous functions that ignore
return values to satisfy golangci-lint errcheck warnings.
|
|
- Add GoDoc comments to AccountManager interface and all YAMLAccountManager methods
- Add complete table-driven test coverage for Create, Update, Get, and List methods
- Tests follow project conventions with proper error handling and temporary directories
- All tests pass with comprehensive verification of both memory state and file operations
|
|
- Convert panic in news article processing to return error instead
- Replace panic in API stats endpoint with HTTP error response
- Update ThreadedNewsMgr interface to return errors from ListArticles
- Ensure server stability by handling errors gracefully
|
|
|
|
Previously, running tests would modify test/config/Users/guest.yaml and create
test/config/Users/test-user.yaml due to the automatic migration logic in
YAMLAccountManager. This caused git diff to show changes after running tests.
Solution:
- Modified TestNewYAMLAccountManager to use t.TempDir() for test isolation
- Added copyTestFiles helper to copy test fixtures to temporary directory
- Tests now run against copies, leaving original fixtures untouched
This ensures tests are properly isolated and don't have side effects on the
repository's test fixture files.
|
|
Replace 60+ hardcoded error strings throughout transaction handlers with
public constants and templates for consistent error handling across the
Hotline protocol implementation.
Features:
- 33 authorization error constants (ErrMsgNotAllowed*)
- Account operation error constants (ErrMsgAccount*)
- File operation error templates with filename parameters (ErrMsgCannot*)
- Upload/download restriction templates (ErrMsgUpload*)
- Ban message constants (ErrMsgTemporaryBan, ErrMsgPermanentBan)
- General error constants (ErrMsgUserNotFound, ErrMsgCreateAlias)
- Chat/messaging templates (ErrMsgDoesNotAcceptTemplate)
Benefits:
- Single source of truth for all error messages
- Public API for other packages to import standard error constants
- Sprintf-style templates for dynamic content (filenames, usernames)
- Clear distinction between protocol errors (ErrMsg*) and golang errors
- Improved maintainability and consistency across transaction handlers
All error messages are now centralized, making future modifications
easier and ensuring consistent user experience across all operations.
|
|
- Document all transaction handler functions with detailed field specifications
- Include request and reply field descriptions with required/optional indicators
- Add field numbers and descriptions for easy protocol reference
- Remove duplicate comments while preserving important context
- Standardize documentation format across all handler functions
|
|
Replace all instances of filepath.Join with path.Join across the codebase
to improve Windows compatibility following the guidance from
https://github.com/golang/go/issues/44305.
Key changes:
- Replaced filepath.Join with path.Join in 14 files
- Updated import statements appropriately
- Resolved variable shadowing issues where function parameters
named 'path' were conflicting with the path package
- Maintained filepath imports where needed for OS-specific functions
like filepath.Walk, filepath.IsAbs, filepath.Dir, and filepath.Base
All tests pass, confirming the changes maintain functionality while
improving cross-platform compatibility.
|
|
Move delete operation before modifying account.Login to prevent deleting wrong key from accounts map.
|
|
|
|
|
|
|
|
|
