Commit Graph

84 Commits

Author SHA1 Message Date
Philip Withnall
e2639ce56e gsocketclient: Fix passing NULL to g_task_get_cancellable()
Fix a regression from commit abddb42d14, where it could pass `NULL` to
`g_task_get_cancellable()`, triggering a critical warning. This could
happen because the lifetime of `data->task` is not as long as the
lifetime of the `ConnectionAttempt`, but the code assumed it was.

Fix the problem by keeping a strong ref to that `GCancellable` around
until the `ConnectionAttempt` is finished being destroyed.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #2687
2022-07-12 16:07:23 +01:00
Philip Withnall
56d371942e gsocketclient: Fix still-reachable references to cancellables
`GSocketClient` chains its internal `GCancellable` objects to ones
provided by the caller in two places using `g_cancellable_connect()`.
However, it never calls `g_cancellable_disconnect()`, instead relying
(incorrectly) on the `GCancellable` provided by the caller being
short-lived.

In the (valid) situation where a caller reuses one `GCancellable` for
multiple socket client calls, or for calls across multiple socket
clients, this will cause the internal `GCancellable` objects from those
`GSocketClient`s to accumulate, with one reference left each (which is
the reference from the `g_cancellable_connect()` closure).

These `GCancellable` instances aren’t technically leaked, as they will
all be freed when the caller’s `GCancellable` is disposed, but they are
no longer useful and there is no bound on the number of them which will
hang around.

For a program doing a lot of socket operations, this still-reachable
memory usage can become significant.

Fix the problem by adding paired `g_cancellable_disconnect()` calls.
It’s not possible to add a unit test as we can’t measure still-reachable
memory growth before the end of a unit test when everything has to be
freed.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #2670
2022-06-14 11:15:18 +01:00
Philip Withnall
5942cd7984 gio: Add SPDX license headers automatically
Add SPDX license (but not copyright) headers to all files which follow a
certain pattern in their existing non-machine-readable header comment.

This commit was entirely generated using the command:
```
git ls-files gio/*.[ch] | xargs perl -0777 -pi -e 's/\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/\n \*\n \* SPDX-License-Identifier: LGPL-2.1-or-later\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/igs'
```

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #1415
2022-05-18 09:18:52 +01:00
Michael Catanzaro
d1e9e0c094 gsocketclient: deprecate tls-validation-flags property
It doesn't work as expected, and you shouldn't be trying to use it
anyway.
2021-11-16 15:21:22 +00:00
Sebastian Dröge
705a59a315 gio: Add missing nullable annotations 2020-11-11 13:15:21 +02:00
Michael Catanzaro
c2b8fa8a34 gsocketclient: fix crash when async connection step fails
This is a regression from !1686. The tmp_error is no longer valid after
it is "considered" and cannot be used at this point. We should print the
error earlier instead.

Fixes #2233
2020-10-28 10:43:43 -05:00
Michael Catanzaro
b88b3712e0 gsocketclient: return best errors possible
Originally, GSocketClient returned whatever error occured last. Turns
out this doesn't work well in practice. Consider the following case:
DNS returns an IPv4 and IPv6 address. First we'll connect() to the
IPv4 address, and say that succeeds, but TLS is enabled and the TLS
handshake fails. Then we try the IPv6 address and receive ENETUNREACH
because IPv6 isn't supported. We wind up returning NETWORK_UNREACHABLE
even though the address can be pinged and a TLS error would be more
appropriate. So instead, we now try to return the error corresponding
to the latest attempted GSocketClientEvent in the connection process.
TLS errors take precedence over proxy errors, which take precedence
over connect() errors, which take precedence over DNS errors.

In writing this commit, I made several mistakes that were caught by
proxy-test.c, which tests using GSocketClient to make a proxy
connection. So although adding a new test to ensure we get the
best-possible error would be awkward, at least we have some test
coverage for the code that helped avoid introducing bugs.

Fixes #2211
2020-10-09 10:50:22 -05:00
Michael Catanzaro
14f7b5e590 gsocketclient: Crash on error if error is missing
We should never return unknown errors to the application. This would be
a glib bug.

I don't think it's currently possible to hit these cases, so asserts
should be OK. For this to happen, either (a) a GSocketAddressEnumerator
would have to return NULL on its first enumeration, without returning an
error, or (b) there would have to be a bug in our GSocketClient logic.
Either way, if such a bug were to exist, it would be better to surface
it rather than hide it.

These changes are actually going to be effectively undone in a
subsequent commit, as I'm refactoring the error handling, but the commit
history is a bit nicer with two separate commits, so let's go with two.
2020-10-09 10:50:22 -05:00
Michael Catanzaro
f0a7b14780 gsocketclient: emit RESOLVING/RESOLVED events only once
GSocketAddressEnumerator encapsulates the details of how DNS happens, so
we don't have to think about it. But we may have taken encapsulation a
bit too far, here. Usually, we resolve a domain name to a list of IPv4
and IPv6 addresses. Then we go through each address in the list and try
to connect to it. Name resolution happens exactly once, at the start.
It doesn't happen each time we enumerate the enumerator. In theory, it
*could*, because we've designed these APIs to be agnostic of underlying
implementation details like DNS and network protocols. But in practice,
we know that's not really what's happening. It's weird to say that we
are RESOLVING what we know to be the same name multiple times. Behind
the scenes, we're not doing that.

This also fixes #1994, where enumeration can end with a RESOLVING event,
even though this is supposed to be the first event rather than the last.
I thought this would be hard to fix, even requiring new public API in
GSocketAddressEnumerator to peek ahead to see if the next enumeration is
going to return NULL. Then I decided we should just fake it: always emit
both RESOLVING and RESOLVED at the same time right after each
enumeration. Finally, I realized we can emit them at the correct time if
we simply assume resolving only happens the first time. This seems like
the most elegant of the possible solutions.

Now, this is a behavior change, and arguably an API break, but it should
align better with reasonable expectations of how GSocketClientEvent
ought to work. I don't expect it to break anything besides tests that
check which order GSocketClientEvent events are emitted in. (Currently,
libsoup has such tests, which will need to be updated.) Ideally we would
have GLib-level tests as well, but in a concession to pragmatism, it's a
lot easier to keep network tests in libsoup.
2020-10-09 10:50:22 -05:00
Michael Catanzaro
290d5722be gsocketclient: document Happy Eyeballs
This isn't an API guarantee, but it's a potentially-surprising
behavior difference between the sync and async functions that is good
to know about, especially because our sync and async functions are
normally identical.
2020-10-09 10:50:22 -05:00
Michael Catanzaro
d971ac7b21 gsocketclient: fix whitespace error 2020-10-09 10:50:22 -05:00
Michael Catanzaro
d24970b207 gsocketclient: fix docs typo 2020-10-09 10:50:22 -05:00
Cristian Rodríguez
35bb69bc47 gsocketclient: set IP_BIND_ADDRESS_NO_PORT if binding to local address
The linux kernel does not know that the socket will be used
for connect or listen and if you bind() to a local address it must
reserve a random port (if port == 0) at bind() time, making very easy
to exhaust the ~32k port range, setting IP_BIND_ADDRESS_NO_PORT tells
the kernel to choose random port at connect() time instead, when the
full 4-tuple is known.
2020-10-09 09:44:05 +01:00
Patrick Griffis
2722620e32 Refactor g_socket_client_connect_async()
This is a fairly large refactoring. The highlights are:

- Removing in-progress connections/addresses from GSocketClientAsyncConnectData:

  This caused issues where multiple ConnectionAttempt's would step over eachother
  and modify shared state causing bugs like accidentally bypassing a set proxy.

  Fixes #1871
  Fixes #1989
  Fixes #1902

- Cancelling address enumeration on error/completion

- Queuing successful TCP connections and doing application layer work serially:

  This is more in the spirit of Happy Eyeballs but it also greatly simplifies
  the flow of connection handling so fewer tasks are happening in parallel
  when they don't need to be.

  The behavior also should more closely match that of g_socket_client_connect().

- Better track the state of address enumeration:

  Previously we were over eager to treat enumeration finishing as an error.

  Fixes #1872
  See also #1982

- Add more detailed documentation and logging.

Closes #1995
2020-02-14 18:15:20 +00:00
Michael Catanzaro
cc3cf6b8b2 gsocketclient: run timeout source on the task's main context
This shouldn't make any difference, because this code should only ever
be running in the main context that was thread-default at the time the
task was created, so it should already match the task's context. But
let's make sure, just in case.
2020-01-07 15:05:22 +00:00
Christian Hergert
22ba4411cc gio: remove use of generic marshaller from GIO objects
Using the generic marshaller has drawbacks beyond performance. One such
drawback is that it breaks the stack unwinding from the Linux kernel due
to having unsufficient data to walk past ffi_call_unixt64. That means that
performance profiling by application developers looks grouped among
seemingly unrelated code paths.

While we can't fix the kernel unwinding here, we can provide proper
c_marshallers and va_marshallers for objects within Gio so that
performance profiling of applications is more reliable.

Related to GNOME/Initiatives#10
2019-06-17 16:29:09 -07:00
Allison Karlitskaya
bdefe5f9e1 gsocketclient: Fix a leak in the connection code
We miss releasing the async operation's reference on a state object in
one of the error cases.

The call to connection_attempt_remove() (although it calls unref
internally) is not sufficient because this is releasing the reference
that the list owns.

Closes #1774
2019-05-09 12:57:00 +02:00
Patrick Griffis
313e7cbad7 gsocketclient: Fix potential critical when cancelling connect
We are manually tracking the completion state of the connect task
so avoid just calling g_task_return_error_if_cancelled() without
checking that.

Fixes #1747
2019-04-16 10:26:01 -07:00
Sebastian Dröge
c29bc822c2 Merge branch 'socket-client-connect-gerror-leak' into 'master'
GSocketClient - Free last error if a connection attempt fails and on retry the...

See merge request GNOME/glib!718
2019-03-15 11:46:57 +00:00
Philip Withnall
1f3375235b docs: Stop formatting integer literals using ‘%’
It makes gtk-doc try to link them.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2019-03-15 11:09:32 +00:00
Sebastian Dröge
499c201472 GSocketClient - Free last error if a connection attempt fails and on retry the cancellable is cancelled 2019-03-12 10:32:56 +01:00
Patrick Griffis
80af199d7d gsocketclient: Fix critical on cancellation
We need to be more explicit in handling cancellation to avoid
multiple task returns.

Fixes #1693
2019-02-27 09:20:28 -05:00
Patrick Griffis
4dd1582625 gsocketclient: Ensure task is always returned on cancel
It was possible for the individual connection attempts to be
cancelled without the main task getting returned.

Fixes libsoup#132
2019-02-09 10:30:24 -05:00
Patrick Griffis
d553d92d6e gsocketclient: Fix criticals
This ensures the parent GTask is kept alive as long as an enumeration
is running and trying to connect.

Closes #1646
Closes #1649
2019-01-29 14:46:52 -05:00
Patrick Griffis
c1e32b9057 gsocketclient: Improve handling of slow initial connections
Currently a new connection will not be attempted until the previous
one has timed out and as the current API only exposes a single
timeout value in practice it often means that it will wait 30 seconds
(or forever with 0 (the default)) on each connection.

This is unacceptable so we are now trying to follow the behavior
RFC 8305 recommends by making multiple connection attempts if
the connection takes longer than 250ms. The first connection
to make it to completion then wins.
2018-12-11 16:09:29 -05:00
Sébastien Wilmet
3bf4a720c3 gio/: LGPLv2+ -> LGPLv2.1+
Sub-directories inside gio/ already processed in a previous commit:
- fam/
- gdbus-2.0/ (which contains only codegen/)
- gvdb/
- inotify/
- tests/
- win32/
- xdgmime/

Other sub-directories inside gio/:
- completion/: no license headers
- kqueue/: not LGPL, BSD-style license

https://bugzilla.gnome.org/show_bug.cgi?id=776504
2017-05-29 19:53:34 +02:00
Christian Hergert
18a33f72db introspection: use (nullable) or (optional) instead of (allow-none)
If we have an input parameter (or return value) we need to use (nullable).
However, if it is an (inout) or (out) parameter, (optional) is sufficient.

It looks like (nullable) could be used for everything according to the
Annotation documentation, but (optional) is more specific.
2016-11-22 14:14:37 -08:00
Piotr Drąg
10c490cdfe Use Unicode in translatable strings
See https://developer.gnome.org/hig/stable/typography.html

https://bugzilla.gnome.org/show_bug.cgi?id=772221
2016-10-12 21:30:42 +02:00
Philip Withnall
3613b7a366 gio: Add source tags to various GTasks constructed in GLib
This makes them easier to identify when debugging and profiling.

This patch was somewhat less than interesting to write.

https://bugzilla.gnome.org/show_bug.cgi?id=767765
2016-06-29 15:16:52 +01:00
Paolo Borelli
9e85f60ec6 socketclient: annotate the connection param of "event" as nullable
When emitting the RESOLVING/RESOLVED events the connection param is
set to NULL.
2015-06-18 16:33:31 +02:00
Dan Winship
6ce79e586f GSocketClient: fix handling of application proxies
g_socket_client_add_application_proxy() claimed "When the indicated
proxy protocol is returned by the #GProxyResolver, #GSocketClient will
consider this protocol as supported but will not try to find a #GProxy
instance to handle handshaking." But in fact, it did the checks in the
wrong order, so GProxy proxies ended up overriding
application-specified ones. Fix that.

Also, simplify the code a bit by making use of g_hash_table_add() and
g_hash_table_contains().

https://bugzilla.gnome.org/show_bug.cgi?id=733876
2015-03-06 16:01:07 -05:00
Philip Withnall
cb320cb5fe gsocketclient: Handle cancellation between CONNECTING and CONNECTED
If a g_socket_client_connect_async() operation is cancelled between the
CONNECTING and CONNECTED events (i.e. while in the
g_socket_connection_connect_async() call), the code in
g_socket_client_connected_callback() would previously unconditionally
loop round and try the next socket address from the address enumerator
(by calling enumerator_next_async()). This would correctly handle the
cancellation and return from the overall task — but not before emitting
a spurious RESOLVING event.

Avoid emitting the spurious RESOLVING event by explicitly handling
cancellation at the beginning of g_socket_client_connected_callback().

https://bugzilla.gnome.org/show_bug.cgi?id=735179
2014-08-22 19:11:26 +01:00
Olivier Crête
14872d2929 socketclient: Fix leak
https://bugzilla.gnome.org/show_bug.cgi?id=726611
2014-03-23 15:00:03 -04:00
Matthias Clasen
d7b9f20990 GSocketClient: convert docs to markdown 2014-02-01 10:22:42 -05:00
Daniel Mustieles
078dbda148 Updated FSF's address 2014-01-31 14:31:55 +01:00
Colin Walters
a46459b000 GSocketClient: For _CONNECTING event, make remote address accessible
My application (hotssh) would like to get the resolved address from DNS,
before we start the connect().

We could add a new event, but it's easy enough to just cache it on the
GSocketConnection; this avoids any new API.

https://bugzilla.gnome.org/show_bug.cgi?id=712547
2013-11-18 17:13:34 -05:00
Emmanuele Bassi
54cc43630d Rename the generated private data getter function
As it turns out, we have examples of internal functions called
type_name_get_private() in the wild (especially among older libraries),
so we need to use a name for the per-instance private data getter
function that hopefully won't conflict with anything.
2013-06-24 15:43:04 +01:00
Emmanuele Bassi
32747def4b gio: Use the new private instance data declaration
Use the newly added macros, and remove the explicit calls to
g_type_class_add_private().

https://bugzilla.gnome.org/show_bug.cgi?id=700035
2013-06-24 14:18:01 +01:00
Dan Winship
c78d0e9ac4 GSocketClient: add missing NULL to g_object_set() call 2013-02-15 10:39:19 -05:00
Dan Winship
7c49869eae GSocketClient: add proxy-resolver property
Add a proxy-resolver property to GSocketClient, to allow overriding
proxy resolution in situations where you need to force a particular
proxy rather than using the system defaults.

https://bugzilla.gnome.org/show_bug.cgi?id=691105
2013-02-14 10:24:14 -05:00
Dan Winship
d21309464c gio: port networking classes from GSimpleAsyncResult to GTask
https://bugzilla.gnome.org/show_bug.cgi?id=661767
2012-10-10 10:29:37 -04:00
Matthias Clasen
6bee6dbce5 Miscellaneous string fixes
Typo and punctuation fixes, and some rewording, based
on a patch by Philip Withnall, bug
https://bugzilla.gnome.org/review?bug=628193
2012-08-16 23:02:41 -04:00
Robert Ancell
4143842eb4 Add missing allow-none annotations for function parameters.
Found using:
find . -name '*.c' | xargs grep 'or %NULL' | grep ' \* @' | grep -v '@error' | grep -v allow-none
2012-03-31 20:34:28 +11:00
David King
5d64eb4cb8 docs: Correct GSocketClient::event link 2012-02-21 01:10:54 +01:00
Benjamin Otte
053b011ccc docs: Clarify GSocketClient reuse policy 2012-02-01 16:25:23 +01:00
Dan Winship
3f3e141ec8 Add GSocketClient::event, for tracking socket client status
This can be used for debugging, or for progress UIs ("Connecting to
example.com..."), or to do low-level tweaking on the connection at
various points in the process.

https://bugzilla.gnome.org/show_bug.cgi?id=665805
2011-12-22 15:44:24 -05:00
Dan Winship
57f279988c Add g_socket_connection_connect(), etc
Previously it was more or less assumed that GSocketConnections were
always connected, although this was not enforced. Make it explicit
that they don't need to be, and add methods to connect them, and
simplify GSocketClient by using those methods.

https://bugzilla.gnome.org/show_bug.cgi?id=665805
2011-12-22 13:22:25 -05:00
Matthias Clasen
f62ee0a0ca GSocketClient: Drop @returns syntax from doc comments 2011-11-21 11:03:31 -05:00
Nicolas Dufresne
cb74c36678 [gio] Terminate connect call when application proxy is used
The connect_async() calls would never terminated when an application side
proxy was being used. Note we also skip over TLS handshake in this case,
as the application may have to do some proxy handshake before.
2011-11-18 12:32:42 -05:00
Nicolas Dufresne
4ae42ceb3c Clear proxy address upon retry
The proxy address was not cleared between each attempt. That would lead
to leak or worse, trying to do the proxy handshake on the final
destination address. To make all this safer, I have regroup all the cleanup
where the iterations starts.

https://bugzilla.gnome.org/show_bug.cgi?id=664141
2011-11-17 23:13:35 -05:00