| Age | Commit message (Collapse) | Author |
|---|
|
Wire an os|memory selector mirroring the existing Redis-vs-file backend
selection. This marks the seam where a future object-store backend (e.g.
Cloudflare R2 / S3) slots in via hotline.WithFileStore.
|
|
Provide a reference non-OS FileStore that models storage as a flat
keyspace of cleaned paths, deriving directory structure from key prefixes
the way an object store does. It proves the FileStore abstraction is
genuinely backend-agnostic and serves as a hermetic test fixture needing
no temp directory. Symlink/ReadLink return errors.ErrUnsupported, matching
how an object-store backend would behave.
Tests drive a full upload -> list -> download round-trip and directory
listing entirely in memory.
|
|
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.
|
|
ListenAndServe previously started each listener in a goroutine that
called log.Fatal on any error, which skipped deferred cleanup and made
errors unobservable to callers, and Server.Shutdown terminated the
process with os.Exit. Context cancellation was also ineffective:
Serve only checked ctx between Accept calls, which block indefinitely.
ListenAndServe now binds its listeners up front and returns bind
errors, closes every listener when the context is canceled so accept
loops unblock and return, and reports the first serve loop error to
the caller. Shutdown closes a lazily-initialized channel that cancels
ListenAndServe's context, so the shutdown API works race-free even
though it starts before ListenAndServe. "Server shutting down" is
logged once by ListenAndServe rather than per accept loop, which
produced duplicate or missing lines depending on scheduling.
main.go now treats context.Canceled as a clean exit, logs other server
errors and exits nonzero, and runs deferred cleanup (e.g. Bonjour
shutdown) on the way out.
|
|
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.
|
|
Anchor the path returned by folderUpload.FormattedPath relative to the
upload root so item paths are resolved consistently, matching the
behavior of ReadPath. Resolve each destination path once per item in
UploadFolderHandler and use path.Join in place of manual string
concatenation.
Add test coverage for FormattedPath normalization.
|
|
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 two unchecked type assertions that could crash the server:
- server.go: the file-transfer logger built remoteAddr via a bare
ctx.Value(contextKeyReq).(requestCtx) assertion, which panics if the
context value is absent or the wrong type. Use the comma-ok form and
degrade to an empty string.
- access.go: the legacy (< v0.17.0) AccessBitmap YAML array path used
byte(v.(int)) with no element-type or bounds check, panicking on a
non-int element or an array longer than the [8]byte bitmap. Iterate the
slice from the type switch, bounds-check the index, and comma-ok each
element, returning a wrapped error so a malformed user file fails to
load loudly instead of crashing the server.
Add regression tests for the non-int and oversized legacy array cases.
|
|
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
|
|
|
|
- Add mutex to protect activeTasks map and connection writes
- Fix nil pointer dereference when handling replies with unknown IDs
- Clean up activeTasks entries after processing replies (memory leak)
- Add done channel to allow keepalive goroutine to exit on Disconnect
- Use io.ReadFull in Handshake to ensure complete reads
- Fix misleading error message when handshake response is unexpected
|
|
|
|
|
|
|
|
|
|
- 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
|
|
Repurpose the previously unused 2-byte field in the tracker protocol to advertise the server's TLS port. This allows clients to discover which servers support TLS connections.
Note: currently only the official, original, 1990's Tracker software sends this server provided 2-byte field to tracker clients. I'm adding this to Mobius in the hope that the modern day trackers might update their behavior to match the original software, which would put these unused 2 bytes to a useful purpose.
|
|
The tracker protocol splits large server lists into batches, with each
batch preceded by a ServerInfoHeader. Previously, GetListing only read
the initial header and failed when encountering subsequent headers
mid-stream, causing the scanner to misinterpret header bytes as server
record data.
Changes:
- Refactored GetListing to use bufio.Reader instead of bufio.Scanner
to handle heterogeneous data (headers and server records)
- Added readServerRecord helper function for sequential reading
- Renamed ServerInfoHeader.SrvCountDup to BatchSize for clarity
- Added comprehensive documentation explaining the batching protocol
- Removed unused serverScanner function and tests
- Added table tests covering multiple batch scenarios
The BatchSize field indicates the number of servers in the current
batch, while SrvCount indicates the total across all batches.
|
|
- Add TLSConfig and TLSPort fields to Server struct
- Add WithTLS option function for configuration
- Add ServeWithTLS and ServeFileTransfersWithTLS methods
- Update ListenAndServe to start TLS listeners when configured
- Add -tls-cert, -tls-key, -tls-port command-line flags
- Fix data race in rateLimiters map access with mutex
- Add TLS documentation with certificate generation instructions
|
|
|
|
|
|
|
|
|
|
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
|
|
|
|
- Replace [4]byte with ForkType in ForkInfoList struct
- Update constants ForkTypeDATA and ForkTypeMACR to use ForkType
- Add String() method to ForkType for better debugging
- Improve consistency with existing type patterns (FieldType, TranType)
- Clean up unused code and improve documentation
|
|
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.
|
|
- Fix critical bug in tracker address parsing: strings.Split(t, ":") instead of strings.Split(":", t)
- Add dependency injection for TrackerRegistrar to improve testability
- Extract registerWithAllTrackers() helper function to eliminate code duplication
- Add comprehensive test suite covering:
* Unit tests for parseTrackerPassword with edge cases and special characters
* Integration tests for registerWithTrackers with mocking
* Context cancellation and graceful shutdown testing
* Error handling for network failures and malformed addresses
* Edge cases like zero ports, long names, and empty configurations
- Improve password parsing to handle colons in passwords correctly
- Add MockTrackerRegistrar for isolated testing without network dependencies
The string splitting bug would have prevented tracker registration from working
with password-protected trackers. All tests pass and verify the fix works correctly.
|
|
- Add table-driven tests for GetNewsArtListData, DataSize, NewsArtListData.Read, NewsArtList.Read, and newsPathScanner
- Improve test coverage from 0% to 87.5%-100% for these functions
- Add test for Field.DecodeNewsPath function
- Fix NewsArtList.Read to return nil instead of io.EOF for proper io.Reader behavior
- Add constants for NewsFlavorCount and improve code documentation
|
|
|
|
- Add tests for UnmarshalYAML covering array and map formats
- Add tests for MarshalYAML with various permission combinations
- Cover edge cases including empty bitmaps and full permissions
- Ensure backward compatibility with legacy array format
|
|
- Add complete test suite for Stats with 100% coverage
- Fix Decrement method to prevent negative values
- Test all methods: Increment, Decrement, Set, Get, Values
- Cover edge cases including zero decrements and mixed operations
|
