sip5060.net / en es
SipMessage: allocate the start line from the message pool Every SipMessage reserved 560 bytes inline for its start line, enough for the larger of RequestLine and StatusLine, although a StatusLine needs only 112. Allocate the actual RequestLine or StatusLine from the message's pool instead. SipMessage goes from 5,128 to 4,568 bytes. A response saves about 450 bytes in total; a request uses the same total, with the start line now in the pool.
Uri: keep password, net namespace and path out of line Almost no URI has a password, a net namespace or a path, but every Uri, and every NameAddr that contains one, reserved room for all three. Move them into a block that is only allocated when the parser or a non-const accessor needs it. The const accessors return Data::Empty while it doesn't exist. Uri goes from 440 to 344 bytes and NameAddr from 608 to 512. Also fix operator==, which compared mPath with itself, so URIs that differed only in path compared equal. Tests: testUri checks parsing, copy, assignment, encoding and equality with and without these parts.
Data: store sizes in 32 bits again, making Data 36 bytes Byron's 2011 change (0cfc0cbb0b) made Data small by storing mSize, mCapacity and mShareEnum as 32-bit values and packing the class to 4-byte alignment. When size_type became size_t (495defc80f) those fields grew to 8 bytes, so Data went from 36 to 48 bytes and the pack no longer saved anything in Data itself. Keep size_type as size_t in the interface, but store the three fields as uint32_t again. Every path that sets a size or capacity now goes through checkedSize(), which throws std::bad_alloc above MaxStoredSize (4 GB - 2), and sums of sizes are computed as size_t so they can't wrap. Growth is capped at MaxStoredSize. setBuf() checks before releasing anything, so it leaves the Data unchanged when it throws. On 64-bit, Data is 36 bytes instead of 48. A parsed INVITE uses about 9% less memory. Because Data is packed to 4-byte alignment, LeakSanitizer can miss pointers to a Data's heap buffer and report it as leaked. Add build/lsan-suppressions.txt for reports about memory allocated during static initialization, and mention it next to ENABLE_ASAN. testParserCategories no longer calls c_str() on the static header and parameter names, which gave them heap buffers at run time. Tests: testData checks the layout, the growth cap and that sizes over the limit throw.
Data, tests: fixes found by the first ASan/UBSan test run - Data: Data is declared with #pragma pack(4), so its size fields can sit at addresses that aren't 8-byte aligned. operator< (three overloads) and operator^= passed them to resipMin/resipMax, which take const references, binding a size_t& to a misaligned member (UBSan: "reference binding to misaligned address", seen in testUri). Pass size() by value instead. - testMultipartRawParts: signedType() parsed a Mime from a local Data and returned it, but the parsed parameters still pointed into that buffer, so later copies read freed memory. Keep the buffer static.
CMake: add ENABLE_ASAN and ENABLE_UBSAN options Both default to OFF and apply to the whole build, libraries included, since a sanitizer only checks code compiled with it. With MSVC, ENABLE_ASAN uses /fsanitize=address and drops /RTC from the Debug flags, which can't be combined with it; ENABLE_UBSAN isn't available there. The OSS-Fuzz build is unaffected: it supplies its own -fsanitize flags.
Stun: close sockets when a STUN binding response is not usable stunOpenSocketPair() returned false without closing its sockets when a binding response failed to parse, losing NUM descriptors and their local ports on every failed attempt. It also ignored getMessage()'s result, parsing whatever a failed read left in the buffer, and did not check that the response carried a MAPPED-ADDRESS. Treat a failed read, a failed parse and a missing MAPPED-ADDRESS the same way, and close all of the pair's sockets before returning false. stunOpenSocket() already closed its socket on a parse failure; it now applies the same read and MAPPED-ADDRESS checks.
build: let the fuzz target stub replay input files Without LIB_FUZZING_ENGINE, fuzz targets link build/oss-fuzz/main.cpp, which only ran the target once on a fixed dummy input. It now runs each file named on the command line through LLVMFuzzerTestOneInput (in an exact-size buffer, so sanitizers catch over-reads), so libFuzzer and OSS-Fuzz crash files can be reproduced and debugged in builds without libFuzzer, such as Visual Studio. With no arguments it keeps the old behaviour.
Parsers: more robustness fixes found by fuzzing - DialogInfoContents: an unrecognized document state or dialog state value parses to the enum's Max sentinel, which encodeParsed() then used to index the string table. Bounds-check all four enum-to-string helpers (dialog-info state, dialog state, state event, direction); out-of-range values encode as an empty string. - XMLCursor: when a document ends right after the root start tag, the constructor's "<root></root>" check looked at bytes beyond the buffer. skipComments() had the same issue with its 4-byte "<!--" lookahead. Check the remaining length before reading. Tests: testDialogInfoContents (unrecognized enumerated values) and testXMLCursor (documents truncated after the root tag or a comment).
Parsers: robustness fixes found by new OSS-Fuzz targets - SipFrag: when a frag's headers are not terminated by a blank line, the header scan ends inside the CRLFCRLF sentinel appended past the frag, so the computed body offset exceeded the frag size and the body length underflowed. Clamp the offset to the frag size so the body never extends beyond the frag, whatever Content-Length claims. - GenericPidfContents: node tag, namespace prefix and value were shared with XMLCursor's buffer. When the document contains comments, XMLCursor parses a comment-stripped copy that it owns, so those values were not valid after parse() returned. Copy them instead. - GenericPidfContents: release a partially built node (and its children) when parsing an element throws, instead of losing it. - Stun: ignore repeated TURN DATA attributes (first one wins), as is already done for REALM, NONCE, ALTERNATE-SERVER and FINGERPRINT. Previously each repeat replaced the stored pointer without freeing it. Tests: testSipFrag (unterminated frag headers with a large Content-Length), testGenericPidfContents (document with comments, malformed elements) and a new rutil testStunParse (repeated TURN DATA, REALM and NONCE; length mismatch).
Merge pull request #505 from kitkatt7/urldecode-trailing-escape rutil: stop Data::urlDecode reading past a trailing '%'
rutil: stop Data::urlDecode reading past a trailing '%' urlDecode guards each %XX escape with i+2 < size(), but after an escape p moves 3 bytes and i only 1. A later '%' near the end passes the stale check, hexpair2int reads past the buffer and p += 2 steps over the end, so the p != end loop never stops. "%41%" is enough. Cookie url-decodes the cookies of a WebSocket upgrade request, so any WS/WSS peer reaches it before authentication. Advance i together with p, as charUnencoded does, and add the cases to testData.
Helper: check digest credentials without parsing the body The server-side digest checks called getContents() for every qop, which can parse the body before the response is compared. Only qop=auth-int uses the body, and it now hashes the received bytes through an opaque wrapper.
Transports: follow-up cleanup after always queuing add/remove (PR #502) Remove SipStack::mProcessingHasStarted. Since transport add/remove and TU registration are always queued through the TransactionController, it is set in getTimeTillNextProcessMS() but never read. Rename TransportSelector::addTransport()'s isStackRunning parameter to queueSharedTransport. The SipStack always passes true, including before the stack runs, so the old name and the "Stack isn't running yet" comment no longer described it. The direct path stays for testTransportSelector, which never runs the process loop. Document in SipStack.hxx that the TransactionController applies addTransport() and removeTransport(), so a transport does not send, receive or start its own thread until the stack is processed. Also note that setCongestionManager() should be called before processing starts: queued transports get the manager in effect when they are added, and the transport list is not locked against a change made while running.
TransportSelector: serialize shared transport registration and removal (#502) * TransportSelector: serialize shared transport registration and removal Queue runtime shared-transport poll registration together with the selector list update, and perform both on the selector thread. This avoids modifying the epoll item array concurrently with event dispatch. Preserve direct registration before processing starts. Use the queued add/remove path when either mInternalThreadsRunning or mProcessingHasStarted is true, including after run() but before the first timer query. Make both flags std::atomic<bool> for cross-thread access. Make Transport::mShuttingDown atomic as well: removal or application shutdown can set it while the selector reads it in basicCheck(). Tag queued requests as ADD or REMOVE instead of inferring the operation from shutdown state. Otherwise, an immediate add/remove can turn both requests into removals and delete the same transport twice. A remove request owns its detached transport and deletes it after unlinking; pending additions remain owned by the transport map. Preserve this ownership when the selector is destroyed with requests still queued. Add testRuntimeTransport to CMake/CTest. * TransportSelector: always queue transport additions and removals Address review feedback. SipStack::addTransport() and removeTransport() now always go through the TransactionController, as TU registration does since #500, so no caller thread changes TransportSelector state. This closes the race with an external EventStackThread that starts before its first timer query. The std::atomic<bool> change to the SipStack running flags is reverted; Transport::mShuttingDown stays atomic. With additions queued, SipStack::setCongestionManager() cannot reach a transport that is still queued. Bind the CongestionManager when the TransactionController handles the AddTransport, using the manager in effect then, instead of in SipStack::addTransport(). SipStack::run() now constructs the TransportSelectorThread and the TransactionControllerThread before starting either. Each constructor changes state the other thread uses: the selector's interruptor, poked by the TransactionController after handling requests queued before run(), and the TransactionController fifo's interruptor, signalled by the selector when it receives. TransportChange is non-copyable, and the Remove branch compares pointers. testRuntimeTransport: registration is deferred until processing in single-thread mode; waits are 10 s and the CTest timeout 120 s; the EventStackThread case adds right after starting the thread. New cases cover adding before run(), setting and clearing the CongestionManager while an addition is queued, and run() after processing while receiving. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add testHupDelivery: a Windows check that a TCP request sent just before
the peer closes is delivered
When a TCP peer sends a request and closes straight away, WSAPoll reports
the data together with POLLHUP. Until the FdPoll change that maps POLLHUP
to Read, the poll group reported that as FPEM_Error, and Connection closed
the socket without reading the request. testFdPollGrp covers the mapping
on POSIX but skips Windows entirely, so nothing exercised WSAPoll.
The test checks three things:
- the raw WSAPoll revents (POLLRDNORM POLLHUP) and the mask FdPollGrp("poll")
passes to the item: Read, not Error, with the data still readable before
EOF
- a SipStack on the poll group, driven by SipStack::process() so that the
request and the FIN are both queued before the stack polls: the request
is delivered and the connection closes on EOF, not as a socket exception
- the same through EventStackThread, 20 times
It runs under ctest on Windows and prints SKIP elsewhere.Merge pull request #501 from RoamerC/fix/epoll-ctl-failures FdPoll: defer opted-in epoll_ctl failures and bound readiness dispatch
TLS: log one line per handshake, and the peer address on failures A successful TLS connection logged four Info lines (handshake starting, connected, session set up, handshake done), and none of them gave the peer's address - "handshake done for peer" only listed the peer certificate's names, which is empty when there is no client certificate. - A completed handshake now logs a single Info line with the mode, peer address, TLS version, cipher, SNI received (server) or target domain (client), and peer certificate names. The handshake starting and session set up lines are now Debug, and the connected lines are gone. - The peer address is now in the handshake failed and certificate name mismatch errors, the OpenSSL error queue dump, and the closed by the peer / read failed lines. The handshake failed error also says whether it was server or client mode, and the target domain in client mode. - Connection's socket exception, connect timeout and write failed logs now include the connection's tuple, not just the socket.
Merge pull request #504 from mykhailopopivniak/upstream-internal-transport-poke Keep other threads off InternalTransport's send buffer
Keep OpenSSL options above bit 31 on Windows; fix MSVC warnings OpenSSL 3 defines SSL_OP_* as 64-bit values, but the parsed option names and the OpenSSLCTXSetOptions / OpenSSLCTXClearOptions settings were held in a long, which is 32 bits on Windows: - SSL_OP_NO_TX_CERTIFICATE_COMPRESSION, SSL_OP_NO_RX_CERTIFICATE_COMPRESSION, SSL_OP_ENABLE_KTLS_TX_ZEROCOPY_SENDFILE and SSL_OP_PREFER_NO_DHE_KEX (bits 32-35) became 0, so setting them in repro or reTurnServer had no effect. - SSL_OP_CRYPTOPRO_TLSEXT_BUG (bit 31) made the long negative, so passing it to SSL_CTX_set_options() sign-extended it and also set bits 32-63 (or cleared them, for OpenSSLCTXClearOptions). parseOpenSSLCTXOption(), BaseSecurity::OpenSSLCTXSetOptions and OpenSSLCTXClearOptions, ReproRunner::setOpenSSLCTXOptionsFromConfig() and ReTurnConfig's option members are now uint64_t. testSecurity checks that bit 31 and bit 32 options parse to their full values. Also fix the other MSVC warnings in the build: - reflow dtls_wrapper tests: TestDtlsUdpSocketContext takes and keeps a resip::Socket rather than an int, and the size_t / uint64_t arguments are cast explicitly. The second test packet used strlen() of the first string. - resip/stack: disable C4267 for the gperf-generated HeaderHash, MethodHash and ParameterHash. gperf's hash() assigns its size_t len to an unsigned int.
Give ParserContainer's HeaderKit real move semantics HeaderKit's copy constructor and copy assignment were a hand-rolled move: they took a const reference, const_cast it, and swapped state with the supposedly const source. Removing HeaderKit::Empty fixed the one shared source, but any copy from a const kit could still write into it. The copy also deep-copied the header text: each time the vector grew, every kit holding raw header text got a new buffer allocated, filled and freed again. Make HeaderKit move-only. Delete the copy operations and add noexcept move construction and assignment. A kit does not own its parser, so a real copy would leave two kits sharing one; copyParsers still does the explicit deep copy. Use emplace_back/emplace for new slots instead of copying a temporary. StlPoolAllocator::construct only accepted const T&, which made std::vector copy elements even when they could be moved. It now forwards its arguments, the standard C++11 allocator form. A ParserContainer built from a received message reserves exactly its header count, so the first push_front or push_back (for example a proxy adding a Via or Record-Route) always grows the vector. That no longer allocates and copies every existing header's text. Add a testSipMessage case that grows a mixed parsed/unparsed container past capacity with push_back and push_front, then checks copy, erase, pop_back and reverse, and that the source is unchanged.
Keep processing TCP write requests after a failed connect processAllWriteRequests() returned after makeOutgoingConnection() failed, so the messages left in mTxFifoOutBuffer waited for the next pass of the transport thread. Continue with the next message instead, as the branch for a missing connection already does.
Check only the shared send fifo in InternalTransport::isFinished() isFinished() is called from the TransactionController thread through TransportSelector::isFinished() while the stack shuts down, and read mTxFifoOutBuffer, which only the transport thread may use. Check mTxFifo instead, as the Transport::isFinished() contract describes.
Explain poke() wake-up rules in one comment Merge the old note on why mSelectInterruptor is not installed in mTxFifo with the note on why poke() does not read mTxFifoOutBuffer.
Merge pull request #503 from mykhailopopivniak/upstream-headerkit-empty Stop copying the shared HeaderKit::Empty into parser containers
Check only the shared send fifo in InternalTransport::poke() poke() runs on the TransactionController thread and asked mTxFifoOutBuffer whether a message was waiting. Only the transport thread may use that buffer: it pops and refills the buffer's deque without a lock. The check was a data race, which ThreadSanitizer reports. Check mTxFifo, which takes its own lock, as poke() did before the buffer was added. Messages already in the buffer need no wake-up: the UDP transport keeps the socket armed for writing while the buffer holds any, and the TCP transports empty the buffer before they wait.
Stop copying the shared HeaderKit::Empty into parser containers HeaderKit's copy constructor and copy assignment move rather than copy: they swap pc and hfv with the source through const_cast. The ParserContainer constructors, push_front, push_back and copyParsers all filled new slots by copying the static const HeaderKit::Empty, so every container built on any thread wrote into that one shared object. ThreadSanitizer reports this as a data race, and writing to a const static is undefined behaviour. Copy from a temporary HeaderKit() instead, so each swap only touches an object local to the call. HeaderKit::Empty has no remaining users and is removed.
FdPoll: defer opted-in epoll_ctl failures and bound readiness dispatch Add addPollItemNotifyOnFailure() for registrations that can clean up on a deferred FPEM_Error. ConnectionManager opts in at registration, and MOD retains that policy. Keep unrecovered ADD/MOD/DEL failures fatal for other registrations; opted-in DEL failures are logged. Register an internal nonblocking, close-on-exec eventfd with epoll so deferred errors also wake select-driven loops. Track the live pending error count and clear the wakeup when no errors remain. Store the item, registration serial and flags together, and tag queued errors with the serial to reject canceled or replaced registrations. Swap the queue into a local batch so callback-generated errors wait for the next call, and preserve undelivered entries if a callback throws. Retry ADD with MOD on EEXIST only when no previous item occupies the slot. Keep live duplicate registration fatal. Log registration over an occupied fd slot, including in release builds, and invalidate cached events and queued errors on registration and removal. Save epoll_ctl errno before logging or handling it. Track registrations known to be absent after failed ADD or MOD/ENOENT, and skip DEL during their expected teardown. If the EEXIST fallback MOD fails for another reason, retain the DEL attempt for the stale entry. Require callers to remove their registration before closing the fd. Map HUP to Read in both poll and epoll so connections can consume final data before EOF. Preserve Error for EPOLLERR, POLLERR and POLLNVAL. Replace the unbounded full-batch loop with one epoll_wait() per call and dispatch its entire returned batch. Share the 200-event batch limit with the regression test. Events outside that batch remain available for subsequent waits, including ET notifications.
Keep registration bindings when a request to remove them fails Previously, any failure response to a removeBinding/removeAll/removeMyBindings request was assumed to mean the bindings were gone, and the usage was destroyed. A rejected removal (for example 401/407 with bad credentials, or any other error) leaves the bindings on the registrar, so ClientRegistration now restores its contact lists, re-arms the refresh timer, sends any queued request, and remains Registered. This applies only when the removal is not ending the registration and a refresh was scheduled. onFailure is still called so the application can retry. The contacts are restored before onFailure is called, so the handler can read them or retry the removal. Based on work by Ben Walden in https://github.com/resiprocate/resiprocate/pull/460, extended to handle all error responses, queued requests, and removals issued from a retry state.
Always queue TU registration through the TransactionController SipStack::registerTransactionUser() inserted the TU into the TuSelector list directly until processing started, and queued it to the TransactionController afterwards. Making that choice safe meant reading mInternalThreadsRunning and mProcessingHasStarted under mAppTimerMutex, which added a lock to every SipStack::processTimers() call and to run() and shutdownAndJoinThreads(). Registration now always goes through the TransactionController's state machine fifo, so only the thread that runs the TransactionController changes the TU list. A TU registered before processing starts is applied on the first TransactionController pass, before any SIP message that could select it, and messages posted to it in the meantime are still delivered through the pending list. Remove the mAppTimerMutex locking added for the choice, and the now unused TuSelector::registerTransactionUser().
Merge pull request #500 from mykhailopopivniak/upstream-tuselector-lock Give the TuSelector list a single owner thread
Fix two thread-safety gaps in source interface lookup WinCompat::instance() used double-checked locking on a plain pointer, so a thread that skipped the lock could see a partly constructed instance. The pointer is now atomic, published with release and read with acquire. destroyInstance() runs at exit after the instance mutex is destroyed, so it swaps the pointer out without taking the lock. lookupRouteSource() threw on a getsockname() failure before unconnecting, leaving the shared route socket connected to the target. It now always unconnects after a successful connect() and reports the getsockname() error afterwards. The cached socket is kept, so a lookup never needs a new descriptor. testConcurrentTransmit assumed the source for 127.0.0.1 is 127.0.0.1. On Windows, WinCompat returns the loopback adapter's first address, which can be another address on that adapter, so no transport matched. The test now registers its transports on any interface and compares every Via with one from a single threaded transmit.
Merge pull request #498 from mykhailopopivniak/upstream-transmit-avg-size Allow concurrent TransportSelector::transmit() after transport setup
Merge pull request #494 from kitkatt7/webadmin-escape-sip-values WebAdmin: escape SIP-supplied URIs and User-Agent in HTML pages
XMLCursor: free a partly parsed element on failure; exact depth limit Follow-up to "Bound nesting depth when parsing XML and multipart bodies". - XMLCursor::parseNextRootChild() leaked the root's child, and every element parsed below it, when parsing that child failed. Now that a document nested past the limit fails on purpose, such documents leaked on every parse. The child is now owned until it is attached to the root. - A child of the root was counted as depth 0, so one more level than XMLCursor::MaxNestingDepth was accepted. It now counts as 1, and MaxNestingDepth is the number of levels allowed below the root element. Tests, based on patches by Nikolai Orekhov: - testXMLCursor: nesting exactly at the limit (walking down to the leaf), one level over it (which fails again on retry), and 20000 unclosed elements. - testMultipartMixedContents: each case with and without Content-Language, both as a standalone body and as the body of an INVITE with Content-Disposition: session after parseAllHeaders(); nesting now goes to 20000 levels.
repro WebAdmin: escape the AOR in the registrations table and remove checkbox
Bound nesting depth when parsing XML and multipart bodies XMLCursor and the multipart body parser went one level deeper on the stack for each level of nesting, with no limit. A body nested many thousands of levels deep could therefore use up the stack of whichever thread parsed it. Real bodies nest only a few levels. rutil: - XMLCursor::Node::skipToEndTag() takes the nesting depth and fails the parse (ParseException) past XMLCursor::MaxNestingDepth (100). Since the tree can be no deeper than that, ~Node() is bounded too. This covers every body parsed with XMLCursor: GenericPidfContents, Pidf, DialogInfoContents and MediaControlContents. resip/stack: - MultipartMixedContents::parse() records each multipart part it creates as one level deeper than itself, and fails a body more than MaxNestingDepth (32) levels below the outermost one (MultipartMixedContents::Exception). The depth is recorded when the part is created, so the limit holds however late the part is parsed, including from tree walks such as Helper::getSdp(), which already catches the exception and stops. A bounded tree also bounds copying and destruction. Copies keep their depth. multipart/alternative, multipart/related and multipart/signed share this parse(). - Contents::preParseHeaders(): Content-Language values are added to the member directly instead of through header(h_ContentLanguages). That accessor calls checkParsed(), so reading the header parsed the part's whole body while its headers were still being read; for a multipart part, every nested level was parsed at once. The other headers handled there already set their members directly. Compatibility: bodies nested deeper than these limits now fail to parse. A nested part carrying Content-Language is now parsed when it is first accessed, like any other part, so an error in its body is reported then rather than when the enclosing body is parsed. MultipartMixedContents gains a data member, so code using it must be rebuilt. Tests: testXMLCursor (a shallow document parses, a deeply nested one fails), testMultipartMixedContents (Helper::getSdp() and a level-by-level walk at 4 levels, at the limit, one level past it and at 1000 levels; parsing the outermost level does not descend into the next one).
WebAdmin: escape SIP-supplied URIs and User-Agent in HTML pages the repro registrations page wrote the User-Agent, Contact URI and Path URIs of each registration straight into the table, while the instance ID next to them was already passed through xmlCharDataEncode. all three are stored as the REGISTER carried them, so a client could put markup in its User-Agent or in a URI parameter and have it run in the admin's browser. the MOHParkServer active calls page had the same problem with the held and invoking URIs taken from INVITE and REFER headers.
Merge pull request #492 from kitkatt7/stun-attr-length-checks
reTurn: log family and length when an address attribute is rejected a valid family with the wrong length now lands in the else branch, and family is a uint8_t, so the old warning printed a raw control character. also note in the test that the short attribute vector only catches a regression under ASan.
Use a keyed hash for hash containers keyed by network data (issue #488) HashMap<Data, ...> hashes with Data::hash(), a fixed Pearson hash with no secret. Anyone can compute it offline and generate thousands of distinct keys with the same hash in seconds. Those keys all land in one bucket, so filling the map becomes quadratic (hash flooding). In testing, 65536 colliding SDP attribute names - about 8 MB, one SIP message over TCP - took 30 s of CPU to insert, against 0.06 s for random names, stalling whichever thread parsed the body. UDP's 64 KB limit keeps it to about 1 ms. rutil: - Data::keyedHash() / keyedCaseInsensitiveHash() (and raw versions): SipHash-1-3, as Python and Rust use for their hash tables, with a 128-bit key chosen randomly once per process. Checked against the SipHash-2-4 reference vectors by running the same code with 2/4 rounds. Values differ between runs, so they must never be stored or sent to another process. - KeyedDataHash: hasher for containers keyed by network data; its comment explains when to use it. - hashCombine(): for composite keys. XOR is unsafe even with keyed parts, since H(x) ^ H(x) is 0 and order is lost (eg. Call-ID == tag). - LinkedHashMap: a hash map that iterates in insertion order, with the commonly used subset of the std::unordered_map interface; Inserter support. - XMLCursor::AttributeMap is now LinkedHashMap<Data, Data, KeyedDataHash>, so attributes are encoded in document order rather than hash order. getAttributes() fails an element with more than 256 attributes (MaxAttributes), and only marks the attributes parsed once parsing succeeds, so a failed parse fails again instead of returning a partial map. resip/stack: - SdpContents: the attribute map uses KeyedDataHash; parsing fails a section with more than 256 distinct attribute names (AttributeHelper::MaxParsedAttributeNames). Repeated names such as candidate share an entry and are not limited. - GenericPidfContents: namespace and attribute maps are LinkedHashMaps with KeyedDataHash, so they encode in document order; the merge's prefix-correction map is keyed too. Pidf::Tuple::attributes matches XMLCursor::AttributeMap. - TransactionMap: branches hash with keyedCaseInsensitiveHash(). resip/dum: - DialogSetId::hash() and DialogId::hash() combine keyed hashes with hashCombine(). This covers DialogUsageManager's DialogSetMap and RedirectManager's RedirectedRequestMap. - DialogUsageManager's CancelMap uses KeyedDataHash. repro: - Proxy's RequestContextMap uses KeyedDataHash. Compatibility: code that names the XMLCursor/PIDF attribute and namespace map types through their typedefs, or uses auto, compiles unchanged; code that wrote out HashMap<Data, Data> for them must switch to the typedefs. Nothing hashed is persisted or sent between processes (RegSync, publication persistence and Record-Route flow tokens use none of these hashes), so redundant repro instances are unaffected. Tests: testData (keyed hash behaviour; 4096 keys sharing one Data::hash() spread out), new testLinkedHashMap, testXMLCursor (document order, attribute limit, retry after failure), testSdp (attribute name limit, 2000 repeated candidates), testGenericPidfContents (encode order, including after a copy), testDSI (swapped and equal parts; 4096 dialog sets that would share one bucket under the old hash).
Merge pull request #491 from kitkatt7/dns-rdata-length-checks rutil/dns: check RDATA length before reading record fields
Docs: fix the Windows doc target and expand RESIP_DEPRECATED in Doxygen CMake: run Doxygen by path in the doc target, not Doxygen::doxygen. The CMAKE_MAP_IMPORTED_CONFIG_* settings at the top of CMakeLists.txt make multi-config generators look up IMPORTED_LOCATION_RELEASE on imported targets, and FindDoxygen only sets IMPORTED_LOCATION on Doxygen::doxygen. Visual Studio configures therefore failed with "IMPORTED_LOCATION not set for imported target Doxygen::doxygen" whenever Doxygen was installed, as on the Windows CI runners. Use DOXYGEN_EXECUTABLE instead. Doxyfile: expand RESIP_DEPRECATED(x) to x (MACRO_EXPANSION with EXPAND_ONLY_PREDEF). Doxygen could not parse declarations wrapped in the macro, and merged them with their neighbours: SipStack's deprecated process(FdSet&) and process(unsigned int) ended up under one entry, for example. The previous commit described this change but left it out.
Docs: build API documentation with Doxygen and publish it to GitHub Pages The API docs link on the wiki (svn.resiprocate.org/dox/) has been dead since the old web server went away (issue #161). Replace the three stale per-component Doxyfiles (repro, resip/dum, resip/stack; doxygen 1.3-1.4 era) with a single top-level Doxyfile that covers rutil, resip/stack, resip/dum, resip/recon, repro, reTurn, reflow and media. - Doxyfile: lists only non-default settings; skips tests, wince and the output of in-source CMake and Visual Studio builds; documents the USE_SSL/USE_DTLS/USE_IPV6 code; and expands RESIP_DEPRECATED(x), which Doxygen could not otherwise parse (it merged SipStack::process(FdSet&) with process(unsigned int), for example). - CMakeLists.txt: optional "doc" target (find_package(Doxygen)), writing HTML to doxygen/html in the build directory; diagrams are dropped if Graphviz is missing. - .github/workflows/docs.yml: builds the docs on every push and pull request, and publishes master to GitHub Pages. Only the resiprocate/resiprocate repository publishes, so forks and private copies never do. - README.md: where to find the docs and how to build them. Documentation pages: - doc/mainpage.dox: new front page linking each component. - rutil and resip/stack main pages: reviewed against the current code and updated (std:: threading typedefs, EventStackThread and SipStackOptions instead of the deprecated StackThread, SipStack::run(), shutdown, and the DTLS/WS/WSS transports), and changed from @mainpage to @page. - New pages for dum, recon, reflow, reTurn, repro and resipmedia. - groups.doc: define the resip_config group that SipStack and StatisticsManager already use; tag Helper with make_sip, which had no members. - reflow: move the FlowManager, Flow and MediaStream class comments onto their classes; they were attached to other declarations and dropped.
reTurn: check STUN attribute lengths before reading address and header stunParseAtrAddress accepted a length of 8 or 20 and then chose how many address bytes to copy from the family byte, so a length-8 attribute with the IPv6 family read 12 bytes past the attribute. stunParseMessage read the 4-byte attribute header with fewer than 4 bytes left. Tie each family to its length and reject a short remainder, as rutil/stun/Stun.cxx does.
rutil/dns: bound record names by RDLENGTH and test the SOA minimum check ares_expand_name stops at the end of the message rather than the end of the record, so an SRV target, NAPTR replacement or CNAME could run on into the next record. check the encoded length against RDLENGTH after expanding. A and AAAA now reject anything but exactly 4 and 16 bytes, as RFC 1035 and RFC 3596 fix RDLENGTH for them. testDnsRecords gains cases for each of these, and feeds truncated SOA records to RRCache::cacheTTL so the RRCache::getTTL check is exercised under AddressSanitizer too. RRMessage is now non-copyable.
dum: honour UserProfile override host/port in out-of-dialog 2xx Contact Helper::makeResponse adds an empty Contact to 2xx responses, which the TransportSelector fills in with the local transport address. ServerOutOfDialogReq never set it from the profile, so OPTIONS responses (answerOptions() and accept()) ignored setOverrideHostAndPort() and GRUUs. For a UAS behind NAT or in a container, that advertises an address the peer cannot reach. Add NonDialogUsage::applyLocalContact(), which builds that Contact from the usage's UserProfile the same way Dialog builds a UAS local Contact: UA capabilities, public or temp GRUU or override host/port, user part from the Request-URI or To, +sip.instance and ;ob. It only replaces the single host-less Contact that makeResponse adds, so a Contact set by the application is kept, and with no override or GRUU the transport still fills in its own address. Call it from ServerOutOfDialogReq::answerOptions() and ServerOutOfDialogReq::accept(). ServerPagerMessage::accept() is unchanged; it already removes the Contact. Add testOptionsContact: a UAC sends OPTIONS to a UAS whose selectUASUserProfile() returns a non-master UserProfile. It covers DUM's automatic answer with and without an override, and an application handler that answers with accept(). It also checks that the OPTIONS Contact uses the override on the UserProfile passed to makeOutOfDialogRequest(), and that neither MasterProfile has an override. Fixes #168
AresDns: report lookups cut short by a DNS server reload; free their payloads When the DNS servers are reloaded and have changed (SipStack::reloadDnsServers(), as repro's WebAdmin and REST admin do), AresDns::init() destroys its channel, and c-ares fails every lookup in flight with ARES_EDESTRUCTION. With c-ares the callback dropped that status without telling the handler or freeing the query's payload, so each such DnsStub query never completed, whatever was waiting on it (a SIP transaction) waited until its own timeout, and the payload leaked. The bundled ares reports these failures. The callback was dropping ARES_EDESTRUCTION for the other case where the channel is destroyed: ~AresDns, when DnsStub has already deleted its queries. ~AresDns now sets a flag first, and each query's payload knows its AresDns, so the callback tells the two apart: during destruction it only frees the payload; on a reload it reports the failure like any other, and DnsStub logs it, tells its sink and frees the query. With the bundled ares, ~AresDns used ares_destroy_suppress_callbacks(), so the payloads of lookups in flight at shutdown were never freed. It now uses ares_destroy() for both resolvers, and the callback frees them. testAresDns checks both: a lookup in flight across a reload is reported once, as failed (the previous c-ares code reported nothing), and one in flight when the resolver is destroyed is not reported. Tested on Ubuntu 22.04, gcc 11, -Werror, against c-ares 1.18.1 and 1.34.5 and with the bundled ares: testAresDns and testDnsPollGrp pass with all three. Under LeakSanitizer, running testAresDns, the previous code leaked two payloads with c-ares and two with the bundled ares; this code leaks none with either.
AresDns: stop using deprecated c-ares calls (#445) c-ares 1.28 deprecated ares_fds(), ares_process() and ares_query(), so building against current c-ares (1.34) warned about all three. With c-ares (WITH_C_ARES) AresDns no longer calls them: - buildFdSet(), used only without a FdPollGrp, filled the fd_sets with ares_fds(). It now builds them from the sockets AresDns already tracks through sock_state_cb, which is installed with or without a group, using the read/write interest c-ares reported for each. - process(fd_set&) called ares_process(). It now hands each ready socket to ares_process_fd(), then calls it once more for timeouts. The ready sockets are collected first, since processing one can close others. - lookup() calls ares_query_dnsrec() on c-ares 1.28 and later. It delivers the reply parsed, and DnsStub parses raw replies itself, so the reply is written back out with ares_dns_write(), as c-ares's own ares_query() does internally since 1.28. The callback runs for every query, failures included, so the payload is always freed. Older c-ares keeps ares_query(). ares_process_fd() is available from c-ares 1.11, the minimum, and is not deprecated. The bundled ares is unchanged. Tested on Ubuntu 22.04, gcc 11, -Werror, against c-ares 1.18.1 and 1.34.5: both build with no warnings (1.34.5 previously gave the three deprecation warnings in #445), and testAresDns and testDnsPollGrp pass with both, including testDnsPollGrp's FdSet-loop cases that use the new buildFdSet() and process() (a refused lookup still completes in 0 ms, failover too, and timeouts fire). The full ctest suite (83 tests, including sanityTests and dumTests) passes against c-ares 1.34.5, where every lookup goes through ares_query_dnsrec(). Fixes #445.
AresDns: close c-ares gaps (socket function, IPv6 servers, ReturnAllResults) Built with c-ares (WITH_C_ARES, the Linux default), AresDns lacked some things it does with the bundled ares: - The application's AfterSocketCreationFuncPtr (typically used to set QoS/DSCP) was never called for the resolver's sockets. It is now called as for SIP TCP connections: before each socket connects (ares_set_socket_configure_callback(), so even a TCP SYN is marked), and again once it is connected if post-connect calls are enabled (ares_set_socket_callback()), for UDP and TCP sockets alike, with the real transport type. - Extra name servers were passed as IPv4 in_addr only; an IPv6 one left an uninitialised entry, so c-ares queried a garbage address. They are now set with ares_set_servers_ports_csv(), IPv4 and IPv6 alike, and a non-zero port is honoured (0 still means 53, which is what repro passes). - checkDnsChange() compared IPv4 addresses only. It now compares the full server lists, so a changed IPv6 server or port is noticed; init() logs IPv6 servers too. The list comes from ares_get_servers_csv() on c-ares 1.24 and later, which deprecated ares_get_servers_ports(), and from ares_get_servers_ports() before that. - ExternalDns::ReturnAllResults set ARES_FLAG_NOCHECKRESP only for the bundled ares. c-ares has the same flag, so DnsStub now sees a refused or failed answer instead of c-ares quietly trying the next server. c-ares 1.11 or later is now required (ares_set_servers_ports_csv and ares_set_socket_configure_callback). None of the new calls is deprecated in current c-ares (see #445). Connection::setEnablePostConnectSocketFuncCall() now also sets ExternalDns::setEnablePostConnectSocketFuncCall(), so the one switch covers DNS sockets as well as SIP TCP connections; the SipStack docs say how DNS sockets are handled. The bundled ares called the socket function for its UDP sockets only; it now calls it for TCP sockets too, likewise once, after connecting. A build without USE_IPV6 does no IPv6, for DNS as well as SIP. With c-ares, IPv6 name servers are now dropped, both extra ones (with the warning the bundled ares already logs) and ones c-ares reads from the system configuration (falling back to 127.0.0.1 if none are left), as the bundled ares does. The bundled ares's own IPv4-only path counted an ignored IPv6 extra name server in nservers but left its in_addr uninitialised, so it queried a garbage address; it now passes only the IPv4 servers, and the system's own if there are none. New rutil/test/testAresDns runs without a network: the socket function is called (with c-ares before connecting, again once connected when enabled, and for a TCP socket after a truncated answer), a ::1 name server is used, checkDnsChange() spots IPv4, IPv6 and port changes (and, without USE_IPV6, ignores IPv6 servers), and with c-ares, ReturnAllResults returns a fake server's REFUSED where c-ares would otherwise skip to a second fake server's answer. Against the previous code every c-ares check fails. Tested on Ubuntu 22.04, gcc 11, -Werror, with c-ares 1.18.1 and with the bundled ares: testAresDns and testDnsPollGrp pass with both, and the full ctest suite passes with c-ares. Both tests also pass against c-ares 1.34.5, where the build shows only the three deprecation warnings #445 reports. With USE_IPV6 off, rutil and both tests build and pass with c-ares 1.18, c-ares 1.34.5 and the bundled ares, and with a resolv.conf listing ::1 and 127.0.0.1 (or ::1 alone), all three use 127.0.0.1 only. With a name server on 127.0.0.1:53 that answers truncated (in a private network namespace), the bundled ares now calls the socket function for its TCP socket as well as the UDP one.
TFM: start each sanityTests/dumTests run with a new log sanityTests and dumTests log to resiprocate.log in their working directory. Log opens that file for appending, so the log kept every run ever made: tfm/repro/resiprocate.log had grown past 250 MB, and finding the latest run in it was hard enough that old runs got mistaken for new ones. - At startup, when logging to a file, each suite renames the previous resiprocate.log to resiprocate.log.old (deleting an older .old first, since rename() on Windows will not replace a file) before Log opens a new one. Only the latest two runs are kept. - Keeping the previous run rather than deleting it matters in CI: ctest --repeat until-pass:2 reruns the whole executable when a suite fails, so the first attempt's log would otherwise be lost. - windows-ci.yml uploads resiprocate.log.old alongside resiprocate.log for both suites, and linux-ci.yml gets the same upload step, with one artifact per compiler in its matrix. - .gitignore: ignore *.log.old (*.log did not cover it).
ares: fail over from a refused server; use a poll group set after init
Two gaps in the bundled ares (rutil/dns/ares), which Windows always
uses and Linux uses with WITH_C_ARES=OFF.
A server that refused a query, or whose socket failed, did not fail the
query over to the next server: handle_error() closed the socket and
marked the server skipped, but its next_server() call was #if 0'd
("seem to corrput memory"), so the query waited out the full timeout.
The corruption was handle_error() walking channel->queries through
query->next after next_server() could have ended and freed the query.
Save next before calling it, and call next_server() again.
Failing over at once exposed a second problem: the next server's socket
can take the descriptor number just closed, look ready to the select()
or poll that ran before, and its read then fails with EAGAIN, which was
taken as a server error. A read that would block is now just nothing
to read.
AresDns only installed the bundled ares poll callback when a FdPollGrp
was set at init(), so a group given later with setPollGrp() never saw
the resolver's sockets and every query timed out. Install it always,
as the c-ares path does; poll items remember their read/write mask, so
moving one to a new group keeps a pending TCP write.
testDnsPollGrp now runs the group-set-after-init cases with the bundled
ares too, requires a refused lookup to finish well inside the timeout
with either resolver, and adds a lookup that must fail over from
127.0.0.1 to 127.0.0.2. Against the previous bundled ares, refused
lookups took the full 5 s timeout and failover about 10 s; now both
finish in under 30 ms.
Tested on Ubuntu 22.04, gcc 11, -Werror, with the bundled ares and with
c-ares, and on Windows with the bundled ares: the full ctest suite
passes on both. testDnsPollGrp passes with each resolver, and the
bundled-ares run is clean under AddressSanitizer.FdPollGrp: use poll() where epoll is not available The poll implementation was only built on Windows (WSAPoll): nothing defined HAVE_POLL. Platforms without epoll, such as macOS and the BSDs, fell back to select(), and resip asserts on any descriptor at or above FD_SETSIZE (1024), so a SIP server there failed at about a thousand connections. CMake now checks for poll() and defines HAVE_POLL, and FdPollGrp::create() picks epoll, then poll, then fdset. - include <poll.h> outside Windows - rename the poll group's mask helpers (CvtPollToUsrMask and CvtUsrToPollMask); they clashed with epoll's once both are built - FdPollImplPoll::waitAndProcess: index pollfds with size_t; the unsigned short wrapped past 65535 descriptors and never ended - report POLLNVAL (a socket closed while still registered) as an error instead of an empty event mask - modPollItem() in the poll and fdset groups ignored descriptor 0, which a daemon that has closed stdin can be given as a socket - SipStack: a stack that creates its own poll group falls back to fdset when the default would be poll, as it already did on Windows, since the poll group cannot serve a select()/process(fdset) loop (StackThread). A SipStack without a group gets what it got before on every platform; code that passes a group or uses EventStackThread gets poll where there is no epoll. New rutil/test/testFdPollGrp covers descriptor 0 in every group and POLLNVAL in the poll group; against the previous code its fdset and poll checks fail. Tested on Ubuntu 22.04, gcc 11, -Werror, c-ares: testFdPollGrp and testDnsPollGrp pass with epoll, poll and fdset. A build with HAVE_EPOLL=0, as on macOS, passes them too, and testStack driven by StackThread works; without the SipStack change it asserts in FdPollImplPoll::buildFdSet(). sanityTests and dumTests pass with their harnesses switched to the poll group. testStack, TCP, 100 sessions: poll completes 20000 transactions where fdset asserts on descriptor 1024.
Merge branch 'master' into dns-rdata-length-checks
Merge pull request #51 from MishaUliutin/master
rutil: test AresDns through each FdPollGrp, offline testDnsPollGrp makes lookups through every FdPollGrp implementation the build has (epoll, fdset, poll), with the group given to DnsStub at init and, for c-ares, set afterwards; through DnsThread taking over an open socket; and through the FdSet select() loop with no group. It needs no network: - a lookup to 127.0.0.1 is refused at once, so with c-ares it must finish well inside the 5 s resolver timeout. A socket that never reached the poll group waits out that timeout instead. - a lookup to 192.0.2.1 (TEST-NET-1) is never answered, so it only finishes if processTimers() drives the resolver timeout. On Linux it also checks that every socket is closed afterwards. The bundled ares sees the refusal but leaves the query to time out (the call to next_server() in handle_error() is disabled), so with it the refused lookup is only checked for completion. Its group-after-init case is skipped: it only installs its poll callback when a group exists at init(). Checked by breaking AresDns on purpose: installing sock_state_cb only when a group exists at init fails the group-after-init cases, and never handing the socket to the group fails every group case.
AresDns/FdPoll: review fixes for the c-ares FdPollItemIf change - c-ares: always install sock_state_cb, so sockets are tracked even when the poll group is set after init(); guard modPollItem() when no group is set - c-ares: processTimers() no longer asserts a poll group, since ares_process_fd() with no sockets only handles timeouts - key the poll item map and AresDnsPollItem by Socket, not int, so SOCKET values are not truncated on Windows - free any remaining poll items in ~AresDns (the bundled ares destroys its channel without callbacks, which leaked them) - FdPollImplEpoll::processFdSet() returns whether epollWait() did anything - drop the unused FdSetIOObserver include and forward declaration, a stray ';' after a namespace, and a stale DnsThread comment Tested on Ubuntu 22.04, gcc 11, -Werror, with c-ares 1.18.1: all 80 ctest tests pass. DNS lookups complete through the epoll and fdset groups, with the group set before or after init, when DnsThread moves the sockets to its own group, with no group at all, and against an unreachable server (timeouts); no sockets leak.
Merge in master and fix merge conflicts
Conflicts in rutil/FdPoll.cxx and rutil/dns/AresDns.{cxx,hxx} were
resolved in favour of the PR, keeping master's (ULONG) cast on the
WSAPoll() call.rutil/dns: check RDATA length before reading record fields The A, AAAA, SRV, NAPTR and SOA parsers read their fixed-size fields without checking the record's RDLENGTH, and DnsStub::createOverlay reads the RR type and length before checking that the fixed part of the RR is inside the message. A response whose last record has a short RDATA makes them read past the end of the message buffer. Reject such records with each type's existing exception, and add testDnsRecords.
tfdum: fix and enable disabled DUM tests Several tests were commented out or under #if 0. Fix each one and enable it: testSessionTimerUasRefresher -> testSessionTimerExpirationUasRefresher The old test slept 90s, past the 60s point where jason (the non-refresher) expires the session. It also never bound the uac object to jason's session, and expected the re-INVITE's 100 Trying from the proxy when there is none in the call. Replace it with a test that sends no refresh, verifies that jason gets onSessionExpired, then ends the call. testDialogEventUacTwoBranchesProvisional The test stopped with two early dialogs still open, so their stale-call timers fired during later tests. Now cancel the call and expect both dialog-event terminations and both Invite_Terminated events. The second fork terminates when its Cancelled timer fires (Timer::TH). Sheila now sends a 100 Trying first, because DumUserAgent::cancelInvite() needs the AppDialogSet it learns in onTrying. testReinviteWithByeSentAfterMissedAck The offerless re-INVITE meant a missing ACK was treated as a rejected offer (no BYE), which testReinviteNo200 already covers. Send the re-INVITE with an offer, so DUM takes the onAckNotReceived path, and expect each 200 retransmission and then the BYE at 32s. Also answer the initial INVITE with SDP, and bind regDerek so the cleanup can unregister derek. testRegisterBasicWithRinstance / testRegisterBasicWithoutRinstance DumUserAgent::makeProfile sets an instance id, and ClientRegistration only uses rinstance when there is none. As a result, WithRinstance could never pass and WithoutRinstance passed whatever the setting. Clear the instance id for the duration of each test. HasRinstance and NoRinstance now check every Contact in the 200, not just the first, so leftover bindings from other tests don't break them. testServiceRoute Per RFC 3608 the service route is not used for REGISTER, which matches DUM's behaviour. Expect the refresh REGISTER at the registrar instead of the service endpoint. testUASPrack2ndOfferInNoAnswerReliableWaitingPrack / testUASPrack2ndOfferInOfferReliableProvidedAnswer ACK the final 406, otherwise the proxy retransmits it into the following tests (it broke testReinviteWithByeSentAfterMissedAck).
TFM: complete a branch before handling the next event; retry failing tests once in CI
testTimerCForked failed on both Linux and Windows CI:
No Sequence expected SipEvent: SIP/2.0 487 Request Terminated
with the test's parent sequence already waiting for exactly that 487.
A finished sequence with no hang-around time queued its
SequenceDoneEvent as a 0 ms timer, and ValueFifo::getNext() queues an
expired timer behind events already waiting. The proxy sends the last
fork its ACK and the caller the 487 almost together. When the 487 was
queued first, it was handled before the And branch counted as finished,
while the parent was still suspended waiting for it.
- SequenceSet::enqueueNext() queues an event to handle before any event
already waiting, and both loops (SequenceSet and SequenceSetThread)
take those first. A sequence with no hang-around time now completes
through it, so its completion takes effect before the next event.
Sequences with a hang-around time still wait it out on a timer.
- Verified by forcing the race (TFM pausing 20 ms after each event):
HEAD fails testTimerCForked 3 of 3 with the error above; with this it
passes. sanityTests (271) and dumTests (95) pass through ctest.
Windows CI also failed testTimerCForkedSequential and
testInviteClientDiesAfterFirstInvite. The TFM logs from that run show
the process stalling, not the tests misbehaving: the proxy processed a
REGISTER 5.6 s after it was sent, and TFM's own 4.25 s timer fired
1.2 s late while an expected retransmission also failed to arrive.
DNS is not the cause: localhost resolved in 2-6 ms per spiral hop in
the same run.
- linux-ci.yml and windows-ci.yml: run ctest with
--repeat until-pass:2, so a failing test runs once more before
failing the job. A genuine regression still fails both runs.