Reference in New Issue
Block a user
Delete Branch "fix/test-coverage"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds unit tests for:\n- #71: sendfile zero-copy\n- #63: multiprocessing module\n- #62: logging module\n- #56: scanner edge cases\n- #55: transport modules
PR Review: fix/test-coverage (
e62296d)Verdict: APPROVE with minor concerns
The PR makes substantial improvements across 41 files (+2112/−180 lines). It adds 12 new test suites (file_sendfile, glob, log, multiprocessing, scanner, transport_ssh/tcp/tls), fixes multiple memory safety and protocol issues, adds symlink support, IPv6 dual-stack, graceful server shutdown, server-side exclude/include filtering, config serialization, and security hardening.
Memory Safety
malloc(0)→ UB indata_create_emptydata.c:7) — allocates at least 1 bytemode_t,uid_t,time_t)metadata.c+metadata.h) — nowint32_t/int64_tfor cross-platform binary protocolcompression.c:14-22) — clamped to[1, 22]> SIZE_MAX→ truncatedmalloccompression.c:83-91) — check added, fallback to growing bufferargv[16]stack buffer overflowtransport_ssh.c:128) — dynamiccalloc(32, ...)with bounds checksbw_throttleelapsed_nspotential overflow (time_t * 1e9)protocol.c:39-41) — usesunsigned long long+ explicit castsbw_throttledeficit_nsnegative →nanosleepwith huge negative valueprotocol.c:53-54) — clamped to 0send_str(NULL)→strlen(NULL)crashprotocol.c:156-159) — NULL check + log + return falsereceive_strno size limit → OOMprotocol.c:173-177) —MAX_STRING_SIZE(10 MB) limitreceive_datano size limit → OOMprotocol.c:206-209) —MAX_DATA_SIZE(1 GB) limitWANT_READ/WANT_WRITEtreated as fatal errorprotocol.c:88-90, 116-118) — retry on non-fatal SSL errorsconfig_createNULL check missing aftermallocconfig.c:18-19)scanner.c:39-89) — avoids use-after-free if caller frees originalsSecurity
link_targetor..)file.c:205-209, 213-217) — blockslink_target[0]=='/'andstrstr(link_target, "..")metadata.c:154) — strips `S_ISUID_exit(0)without cleanupserver.c:157-158) — usesserver_request_shutdown()for graceful cleanupfile_save_to_diskweak path traversal checkstrstr(disk_path, "..")which has false positives (valid..name) and potential bypasses (.../.../). Considerrealpath()+ prefix check.--ca, but without it,SSL_VERIFY_NONElogs a warning only (transport_tls.c:79-81). Acceptable for tooling.Thread Safety
io_read_fd,io_write_fd,io_ssl,io_bwlimitare__threadg_tcp_cleanup_requestedisvolatile sig_atomic_tg_server_cleanup_requestedsame patternaccept_loopusesSIGCHLD=SIG_IGNLogic / Correctness
receive_delta_file: unexpectedresp(neitherDELTA_DATAnorNEXT) silently returns NULL withoutSTATUS_ERRORreceive_incremental_check:send_status(STATUS_OK)failure on match returns NULL withoutSTATUS_ERRORis_excludedon server createsstr_dup+basename()for every filebasenamemodifies input, so duplication is necessaryfile_save_to_disk:strstr(disk_path, "..")is notNULL-safedisk_pathis never NULL at that pointCode Quality
goto error/ early return frees all prior allocationsconfig_receiveoverflow guardSIZE_MAX / sizeof(char*)check prevents integer overflow in mallocMAX_PATTERN_COUNT= 10000server_createtriesAF_INET6first, falls back toAF_INET4server_accept_looppublicclient_connectusesgetaddrinfoinet_ptonFileTypeenum,link_targetfield,FILE_TYPE_SYMLINKhandling in scanner/send/receive/saveTest Coverage Analysis
test_file_sendfile,test_glob,test_log,test_multiprocessing,test_scanner,test_transport_ssh,test_transport_tcp,test_transport_tlsmalloc(0)data, oversized string rejection, truncated read, NULL destroy, empty directory scanner, globstar patterns, compression fallbackdirectory_scanner_create_fullwithfollow_symlinks=false, partial file recovery,io_set_bwlimit/bw_throttle, server-sideis_excluded,delete_extras/receive_manifest,file_save_to_disksymlink path traversal blocking,file_send_streamingerror paths,receive_delta_file,send_str(NULL)regressionSummary
Re-review: FIX VERIFIED ✅
Verdict: APPROVED
Pull request closed