- 22 Sep, 2014 5 commits
-
-
Philip Withnall authored
Rather than passing them through to the application. Invalid STUN packets are still passed through, but it causes unnecessary noise and corruption for higher-level applications to receive STUN packets they weren’t expecting. This is permitted by RFC 5389, §7.3.0: If any errors are detected, the message is silently discarded. In the case when STUN is being multiplexed with another protocol, an error may indicate that this is not really a STUN message; in this case, the agent should try to parse the message as a different protocol. where I interpret ‘error’ to mean ‘validation error’ rather than (e.g.) ‘unrecognised attribute’ or ‘unmatched response’ where the STUN packet is otherwise perfectly formed.
-
Philip Withnall authored
If a NiceSocket is destroyed, various pointers are currently left dangling to it in the conncheck state. These can cause crashes if (for example) a CandidateCheckPair with such a dangling pointer is then used; the GSocket methods will fail. Fix this by explicitly removing the socket and all NiceCandidates which wrap it from various areas of the state.
-
Philip Withnall authored
This introduces no functional changes.
-
Philip Withnall authored
-
Philip Withnall authored
To allow enhanced grepping for what structs point to other structs.
-
- 19 Sep, 2014 2 commits
-
-
Philip Withnall authored
Modify agent_timeout_add_with_context() to force destroying and freeing of an existing GSource before overwriting it with a new one (probably with an updated timeout period). This fixes a case in priv_map_reply_to_relay_refresh() where the TURN candidate refresh timer was being overwritten with a new one, without the old one being destroyed. This lead to two timeouts existing, only one of which would be destroyed when the CandidateRefresh struct was freed, leaking the other one (in the main context) and allowing it to be later dispatched with a dangling CandidateRefresh pointer. The modification to agent_timeout_add_with_context() should prevent this happening in new code in future.
-
Philip Withnall authored
Modify the agent_timeout_add_with_context() utility function to automatically add names to the timer GSources it creates. This makes them a little easier to identify when debugging.
-
- 02 Sep, 2014 2 commits
-
-
Philip Withnall authored
Explicitly check whether the socket is closed (universally represented as sock->priv == NULL) before doing anything else in the socket methods. This should safely return from unusual situations where the socket has been closed and part-destroyed but still ends up having send() or recv() called on it.
-
Philip Withnall authored
The return value is the number of messages sent, or -1 on error — not a boolean.
-
- 01 Sep, 2014 4 commits
-
-
Philip Withnall authored
Since dropping a STUN message due to having insufficient buffer space in libstun can cause ICE negotiation to fail. So we want it to be more obvious in the logs.
-
Olivier Crête authored
Otherwise we risk running out of space in the space limited saved ids list. So the easiest way to do that is to put one StunAgent in each component. It may be advisable to just give up on not allocating memory inside libstun and just use a regular hash table in there instead.
-
Olivier Crête authored
-
Philip Withnall authored
The agent can reach EOS after performing one or more non-empty reads in its read loop, and then hitting EOS on the underlying stream. That means reached_eos is TRUE, but n_valid_messages is non-zero. Weaken the postcondition to reflect this.
-
- 27 Aug, 2014 2 commits
-
-
Philip Withnall authored
As mentioned in the previous commit message, component_free() requires the agent lock to be released, but component_close() requires it to be held. Fix the locking in nice_agent_remove_stream() to do that.
-
Philip Withnall authored
Closing a component and freeing it are slightly conceptually different. As it happens, freeing it can result in the disposal of the component's NiceIOStream, which requires the agent lock to be released — but closing the rest of the Component requires the agent lock to be held. Splitting the function up simplifies locking handling in other parts of the code (see next commit).
-
- 25 Aug, 2014 3 commits
-
-
Philip Withnall authored
The mutex is now non-recursive. This is left over from commit 1deee693.
-
Philip Withnall authored
From commit 7b6935c6.
-
Philip Withnall authored
Bad merge in commit 67179ffd.
-
- 23 Aug, 2014 2 commits
-
-
Philip Withnall authored
This got lost in rebasing the FIN–ACK work. component->tcp is now only NULL before the pseudo-TCP stream is initially created — afterwards, it is always non-NULL, but pseudo_tcp_stream_is_closed() returns TRUE if the stream has closed (gracefully or on errors). This allows differentiation between the states: TCP support was never initialised; and TCP support was initialised but is now closed.
-
Philip Withnall authored
We should return 0 instead of G_IO_ERROR_CLOSED for consistency with recv() and read().
-
- 21 Aug, 2014 20 commits
-
-
Philip Withnall authored
Add a few safeguards to ensure that once priv->shutdown is set, it is not overwritten with a different value.
-
Philip Withnall authored
Change pseudo_tcp_socket_recv() to return 0 if in non-FIN–ACK mode and the socket has been shut down. This makes the behaviour of FIN–ACK and non-FIN–ACK modes consistent. This is a behaviour change from before the introduction of FIN–ACK mode — previously, pseudo_tcp_socket_recv() would return -1 and set the ENOTCONN error if called after the socket had been shut down (i.e. after pseudo_tcp_socket_get_next_clock() had returned FALSE). The new behaviour will hopefully not break anything.
-
Philip Withnall authored
If the socket is closed (with pseudo_tcp_socket_close()) with FIN–ACK unsupported, it should transition to the CLOSED state when pseudo_tcp_socket_get_next_clock() returns FALSE (as that’s the old-style indication that the socket has closed). This is a behaviour change from before the introduction of FIN–ACK: previously, the socket would stay in the ESTABLISHED state. However, this should not have been easily detectable by consumers of the API, as pseudo_tcp_socket_is_closed() did not exist then.
-
Philip Withnall authored
This allows FIN–ACK support to be disabled entirely. This is mostly for testing purposes, since TCP_OPT_FIN_ACK is negotiated when establishing the connection, and is disabled if the other side doesn’t support it. This includes an interoperability test.
-
Philip Withnall authored
As found by tsan.
-
Philip Withnall authored
This means that users of the Nice[Input|Output|IO]Stream API can easily close TCP connections without having to hack around with libnice internals.
-
Philip Withnall authored
-
Philip Withnall authored
This is analogous to the UNIX shutdown() function, allowing either or both sides of a pseudo-TCP connection to be shut down.
-
Philip Withnall authored
The TIME-WAIT timeout is typically 2×MSL (on the order of 60 seconds), which is needed to be able to reject delayed segments from closed conversations. However, the underlying socket layer for pseudo-TCP takes care of channel numbering so that segments don’t end up being sent to closed conversations. Therefore, the TIME-WAIT state can be eliminated (by shortening the timeout). The code for the state is kept around so that the pseudo-TCP implementation matches the TCP specification correctly, which will simplify maintenance.
-
Philip Withnall authored
This explicitly removes all timers and main loop considerations from the pseudo-TCP testing, which considerably simplifies arranging packet swaps and mistimings for testing purposes. This test suite includes a few tests for the FIN–ACK support.
-
Philip Withnall authored
In order to detect cases where the peer closes its connection without an explicit in-band close message (e.g. in protocols such as Telnet where there is none), pseudo-TCP needs to grow support for a shutdown handshake, following the TCP FIN–ACK specification. Arguably it should have had this all along, but Jingle apparently doesn’t need it. This adds support for FIN–ACK to the pseudo-TCP implementation. It is backwards-compatible, only being used if the TCP_OPT_FIN_ACK option is specified in the SYN segment. If enabled, full-duplex closes are supported, and the standard method for notifying a peer of the other end closing its connection (returning 0 from recv()) is used. Also allow rapidly tearing down a connection, discarding unsent and unreceived data, by sending an RST segment. This preserves the ability to do a forced socket closure with pseudo_tcp_socket_close(sock, TRUE). It also permits graceful socket shutdown in the case where the final ACK is lost, and one peer gets stuck in the LAST-ACK state: that peer will eventually re-transmit its FIN segment. The other peer, in the CLOSED state, will respond with a RST segment, and the first peer will then reach CLOSED. References (most useful first): • http://tools.ietf.org/html/rfc793#section-3.5 • http://tools.ietf.org/html/rfc1122#page-87 • http://vincent.bernat.im/en/blog/2014-tcp-time-wait-state-linux.html • http://tools.ietf.org/html/rfc675 Diagram: • http://en.wikipedia.org/wiki/Transmission_Control_Protocol#mediaviewer/File:TCP_CLOSE.svg
-
Philip Withnall authored
As with send(), pseudo_tcp_socket_send() may return EPIPE if the local side of the connection has been closed using close() or shutdown(). It currently doesn’t, but will do once pseudo-TCP FIN–ACK support has been implemented.
-
Philip Withnall authored
Instead, keep the closed object around. This allows differentiation between the states: TCP support was never initialised; and TCP support was initialised but is now closed.
-
Philip Withnall authored
-
Philip Withnall authored
Both state changes need some follow-up code executed immediately after, which was duplicated in a number of places. Factor that out. This introduces no behavioural changes.
-
Philip Withnall authored
This clarifies the code a little, and does not introduce functional changes.
-
Philip Withnall authored
This removes some hard-coded offsets. It does not introduce any functional changes.
-
Philip Withnall authored
Rather than reconstructing the flag state when sending a segment, just store the flags explicitly. This does not introduce any behavioural changes.
-
Philip Withnall authored
This is needed for the upcoming new test suite for pseudo-TCP. It shouldn’t be used in normal code — only in tests. Ideally, the pseudo-TCP code should originally never have called g_get_monotonic_time() itself, and should have always taken a time parameter from the caller; then it would be more testable. Unfortunately, API guarantees prevent this from being changed now.
-
Philip Withnall authored
This should make debugging a little clearer.
-