gio/gcredentials.c: In function ‘linux_ucred_check_valid’:
gio/gcredentials.c:317:22: error: comparison of integer expressions of different signedness: ‘uid_t’ {aka ‘unsigned int’} and ‘int’
317 | || native->uid == -1
| ^~
gio/gcredentials.c:318:22: error: comparison of integer expressions of different signedness: ‘gid_t’ {aka ‘unsigned int’} and ‘int’
318 | || native->gid == -1)
| ^~
gio/gcredentials.c: In function ‘g_credentials_set_unix_user’:
gio/gcredentials.c:639:29: error: comparison of integer expressions of different signedness: ‘uid_t’ {aka ‘unsigned int’} and ‘int’
639 | g_return_val_if_fail (uid != -1, FALSE);
| ^~
Split out XDG_CURRENT_DESKTOP handling to a separate function and make
sure that it drops all the invalid entries properly. Earlier a bad
entry could slip through the checks by sitting just after another bad
entry, like in env being set to `invalid1!:invalid2!`, where
`invalid2!` could slip the checks.
It occasionally fails in CI with output like:
```
196/274 glib:gio / gdbus-connection-slow FAIL 0.54 s (killed by signal 6 SIGABRT)
--- command ---
G_TEST_BUILDDIR='/builds/pwithnall/glib/_build/gio/tests' G_TEST_SRCDIR='/builds/pwithnall/glib/gio/tests' GIO_MODULE_DIR='' /builds/pwithnall/glib/_build/gio/tests/gdbus-connection-slow
--- stdout ---
\# random seed: R02S4eb186e89e2472eedd11538b37192543
1..2
\# Start of gdbus tests
\# Start of connection tests
Bail out! GLib-GIO:ERROR:../gio/tests/gdbus-connection-slow.c:98:test_connection_flush: assertion failed (error == NULL): Child process killed by signal 11 (g-exec-error-quark, 19)
--- stderr ---
**
GLib-GIO:ERROR:../gio/tests/gdbus-connection-slow.c:98:test_connection_flush: assertion failed (error == NULL): Child process killed by signal 11 (g-exec-error-quark, 19)
cleaning up pid 12991
```
which is not very helpful. Add some more debug output to print the
stdout and stderr of the child process, to hopefully give an insight
into why it’s dying with signal 11 (sigsegv).
I can’t reproduce the sigsegv locally.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
As with previous commits, this could have been used to load private data
for an unprivileged caller.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2168
It could have been used to load private data which would not normally be
accessible to an unprivileged caller.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2168
Its components are used to build filenames, so if the value of
`XDG_CURRENT_DESKTOP` comes from an untrusted caller (as can happen in
setuid programs), using it unvalidated may be unsafe.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2168
As with the previous commit, it’s unsafe to trust the environment when
running as setuid, as it comes from an untrusted caller. In particular,
with D-Bus, the caller could set up a fake ‘system’ bus which fed
incorrect data to this process.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2168
Even if the modules in the given directory never get chosen to be used,
loading arbitrary code from a user-provided directory is not safe when
running as setuid, as the process’ environment comes from an untrusted
source.
Also ignore `GIO_EXTRA_MODULES`.
Spotted by Simon McVittie.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2168
Clang says:
../gio/glocalfile.c:2090:11: warning: variable 'success' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (trashdir == NULL)
^~~~~~~~~~~~~~~~
../gio/glocalfile.c:2133:12: note: uninitialized use occurs here
if (!success)
^~~~~~~
../gio/glocalfile.c:2090:7: note: remove the 'if' if its condition is always true
if (trashdir == NULL)
^~~~~~~~~~~~~~~~~~~~~
../gio/glocalfile.c:2041:23: note: initialize the variable 'success' to silence this warning
gboolean success;
^
= 0
So just do that.
Most variables were, but a few were not declared as local, and hence
leaked into the calling environment every time someone tab-completed the
`gio` command.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2275
- When querying a TCP socket, getsockopt() may succeed but the resulting
`optlen` will be zero. This means we'd previously be reading
uninitialized stack memory in such cases.
- After a file-descriptor has gone through FD-passing, getsockopt() may
fail with EINVAL. At least this is the case with TCP sockets.
- While at it also use SOL_LOCAL instead of hard-coding its value.
Contrary to what the WSARecvFrom seem to imply, a UDP socket is perfectly recoverable and usable after a WSAECONNRESET error (and, I assume, WSAENETRESET).
However GSocket condition has the FD_READ bit set after a UDP socket fails with WSAECONNRESET, even if no data is available on the socket anymore; this causes select calls to report the socket as readable when, in fact, it's not.
The change resets FD_READ flag on a socket upon the above error conditions; there's no 'if' to filter between datagram and stream sockets as the change should be harmless in the case of stream sockets which are, however, very unlikely to be usable after a WSAECONNRESET.
The list is sorted in ascending order, which means that to put
verbs alphabetically we need to sort ealier verbs with -1. Same for
the "open" verb and the preferred verb (if any).
* UWP apps that have low registry footprint might end up with chosen_handler == NULL.
Ensure that this doesn't happen.
* UWP apps don't need verbs for URIs, but we do need verbs to have a link to an app
(since handlers don't contain app fields). Work around this by adding an "open" verb
to each UWP URI handler.
* Duplicate the code that inserts extension handler verbs into the app to also insert
URI handler verbs. This allows URI-only apps to be used correctly later on (otherwise
GLib errors out, saying that the app has no verbs).
Use pretty name as the result of _name(), if available. This is
more in line with what .desktop files return. Canonical name
may be completely unintelligible.
MSDN doesn't say much on this subject, but i've seen apps in the wild
that have the "shell" subkey with verbs *either* in the root app key *or*
in the "Capabilities" subkey of the root key. Accommodate either case by trying both
(root key gets a priority, since this is how MS Address Book is registered -
assume that MS knows how to do this the right way).
This function enumerates all user-accessible UWP packages
and calls the user-provided callback for each package.
This can be used to make GLib aware of the UWP applications
installed in the system.
The function works by using IPackageManager/IPackage UWP interfaces
and XmlLite COM library to parse package manifests.
The function requires COM, and initializes it to a single-thread
appartment model. To ensure this doesn't break anything, either
only use it in a separate thread (COM is initialized on a per-thread
basis), or make sure that the main thread also uses the same COM
model (it's OK to initialize COM multiple times, as long as the same
model is used and as long as init/uninit calls are paired correctly).
MinGW-w64 lacks the appropriate headers, so we have to add them
here. Note that these only have the C versions (normally these
things come in both C and C++ flavours), since that's what we use.
Also note that some of the functions that we don't use (but must
describe to maintain binary compatibility) were altered to use
IUnknown (basically, an untyped pointer) instead of the appropriate
object types, as adding these types would require other types,
which would pull even more types, forcing us to drag half of the
UWP headers in here. By replacing unused types with IUnknown we
can trim a lot of branches from the dependency graph.
This is a COM object that implements IStream by using a HANDLE
and WinAPI file functions to access the file (only a file; pipes
are not supported). Only supports synchronous access (this is
a feature - the APIs that read from this stream internally will
never return the COM equivalent of EWOULDBLOCK, which greatly
simplifies their use).
Just embed a PNG instead. gdk-pixbuf deprecated its pixdata support in
version 2.32, in 2015.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #1281
gio/gcredentials.c: In function ‘g_credentials_to_string’:
gio/gcredentials.c:238:31: error: comparison of integer expressions of different signedness: ‘uid_t’ {aka ‘unsigned int’} and ‘int’
238 | if (credentials->native.uid != -1)
| ^~
gio/gcredentials.c:240:31: error: comparison of integer expressions of different signedness: ‘gid_t’ {aka ‘unsigned int’} and ‘int’
240 | if (credentials->native.gid != -1)
| ^~
gio/gfileattribute.c: In function ‘escape_byte_string’:
gio/gfileattribute.c:286:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’}
286 | for (i = 0; i < len; i++)
| ^
gio/gfileattribute.c:299:21: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’}
299 | for (i = 0; i < len; i++)
| ^
gio/gicon.c: In function ‘g_icon_to_string_tokenized’:
gio/gicon.c:165:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
165 | for (i = 0; i < tokens->len; i++)
| ^
gio/gfileinfo.c: In function ‘g_file_info_remove_attribute’:
gio/gfileinfo.c:706:9: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
706 | if (i < info->attributes->len &&
| ^
Fix signedness warning in gio/gfileinfo.c:g_file_info_create_value()
gio/gfileinfo.c: In function ‘g_file_info_create_value’:
gio/gfileinfo.c:1084:9: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
1084 | if (i < info->attributes->len &&
| ^
Fix signedness warning in gio/gfileinfo.c:matcher_matches_id()
gio/gfileinfo.c: In function ‘matcher_matches_id’:
gio/gfileinfo.c:2624:21: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
2624 | for (i = 0; i < matcher->sub_matchers->len; i++)
| ^
Fix signedness warnings in gio/gfileinfo.c:g_file_attribute_matcher_enumerate_namespace()
gio/gfileinfo.c: In function ‘g_file_attribute_matcher_enumerate_namespace’:
gio/gfileinfo.c:2713:21: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
2713 | for (i = 0; i < matcher->sub_matchers->len; i++)
| ^
gio/gfileinfo.c:2715:27: error: comparison of integer expressions of different signedness: ‘guint32’ {aka ‘unsigned int’} and ‘int’
2715 | if (sub_matchers[i].id == ns_id)
| ^~
Fix signedness warning in gio/gfileinfo.c:g_file_attribute_matcher_enumerate_next()
gio/gfileinfo.c: In function ‘g_file_attribute_matcher_enumerate_next’:
../glib.git/gio/gfileinfo.c:2752:13: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’} [-Werror=sign-compare]
2752 | if (i < matcher->sub_matchers->len)
| ^
gio/gfileinfo.c: In function ‘g_file_info_list_attributes’:
gio/gfileinfo.c:645:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
645 | for (i = 0; i < info->attributes->len; i++)
| ^
gio/gfileinfo.c: In function ‘g_file_info_has_namespace’:
gio/gfileinfo.c:610:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
610 | for (i = 0; i < info->attributes->len; i++)
| ^
gio/gfileinfo.c: In function ‘g_file_info_find_value’:
gio/gfileinfo.c:543:9: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
543 | if (i < info->attributes->len &&
| ^
gio/gfileinfo.c: In function ‘g_file_info_clear_status’:
gio/gfileinfo.c:499:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
499 | for (i = 0; i < info->attributes->len; i++)
| ^
gio/gfileinfo.c: In function ‘g_file_info_set_attribute_mask’:
gio/gfileinfo.c:453:21: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
453 | for (i = 0; i < info->attributes->len; i++)
| ^
gio/gfileinfo.c: In function ‘g_file_info_copy_into’:
gio/gfileinfo.c:385:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
385 | for (i = 0; i < dest_info->attributes->len; i++)
| ^
gio/gfileinfo.c: In function ‘g_file_info_finalize’:
gio/gfileinfo.c:327:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
327 | for (i = 0; i < info->attributes->len; i++)
| ^
gio/gdatainputstream.c: In function ‘scan_for_chars’:
gio/gdatainputstream.c:879:40: error: comparison of integer expressions of different signedness: ‘int’ and ‘gsize’ {aka ‘long unsigned int’}
879 | for (i = 0; checked < available && i < peeked; i++)
| ^
gio/gdatainputstream.c: In function ‘scan_for_newline’:
gio/gdatainputstream.c:654:40: error: comparison of integer expressions of different signedness: ‘int’ and ‘gsize’ {aka ‘long unsigned int’}
654 | for (i = 0; checked < available && i < peeked; i++)
| ^
gio/gliststore.c: In function ‘g_list_store_insert’:
gio/gliststore.c:272:30: error: comparison of integer expressions of different signedness: ‘guint’ {aka ‘unsigned int’} and ‘gint’ {aka ‘int’}
272 | g_return_if_fail (position <= g_sequence_get_length (store->items));
| ^~
gio/gliststore.c: In function ‘g_list_store_splice’:
gio/gliststore.c:482:21: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
482 | for (i = 0; i < n_additions; i++)
| ^
gio/gcancellable.c:773:1: error: missing initializer for field ‘closure_marshal’ of ‘GSourceFuncs’ {aka ‘struct _GSourceFuncs’}
773 | };
| ^
In file included from glib/giochannel.h:33,
from glib/glib.h:54,
from gio/gcancellable.c:22:
glib/gmain.h:277:23: note: ‘closure_marshal’ declared here
277 | GSourceDummyMarshal closure_marshal; /* Really is of type GClosureMarshal */
| ^~~~~~~~~~~~~~~
gio/gcontextspecificgroup.c: In function ‘g_context_specific_source_new’:
gio/gcontextspecificgroup.c:77:3: error: missing initializer for field ‘closure_callback’ of ‘GSourceFuncs’ {aka ‘struct _GSourceFuncs’}
77 | };
| ^
In file included from glib/giochannel.h:33,
from glib/glib.h:54,
from gobject/gbinding.h:28,
from glib/glib-object.h:22,
from gio/gcontextspecificgroup.h:23,
from gio/gcontextspecificgroup.c:22:
glib/gmain.h:276:19: note: ‘closure_callback’ declared here
276 | GSourceFunc closure_callback;
| ^~~~~~~~~~~~~~~~
gio/inotify/inotify-kernel.c: In function ‘ik_source_new’:
gio/inotify/inotify-kernel.c:377:3: error: missing initializer for field ‘finalize’ of ‘GSourceFuncs’ {aka ‘struct _GSourceFuncs’}
377 | };
| ^
In file included from glib/giochannel.h:33,
from glib/glib.h:54,
from gio/inotify/inotify-kernel.c:30:
glib/gmain.h:272:14: note: ‘finalize’ declared here
272 | void (*finalize) (GSource *source); /* Can be NULL */
| ^~~~~~~~
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
The version of `black` on the CI server wanted these changes. Make them
to keep the `style-check-diff` CI job from constantly failing.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Try and catch programmer errors in third-party implementations of
`dbus_register()`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #1188
NULL is valid return value for the g_unix_mount_get_options function
because mount options are currently provided only by libmount implementation.
However, the gio tool passes the returned value to the g_strescape function
without checking, which produces the following critical warning:
GLib-CRITICAL **: 13:47:15.294: g_strescape: assertion 'source != NULL' failed
Let's add the missing check to prevent the critical warnings.
These might help catch the problem in #2119 earlier on, and provide more
information about its root cause.
They should not affect behaviour in normal application usage.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2119
This is technically an API break, as the following assignment may now
raise warnings in user code:
```
gchar *filename = g_osx_app_info_get_filename (app_info);
```
However, from code search it seems like the number of users of that
function is zero.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This command will try to execute a desktop file, before that
it will load the input as a keyfile for checking its existence
and its validity (as a keyfile).
File arguments are allowed after the desktop file.
Closes#54
Signed-off-by: Frederic Martinsons <frederic.martinsons@sigfox.com>
Static analysis of the call to `g_dir_new_from_dirp()` is tricky,
because the call is across library boundaries and indirected through a
vfunc map because it’s private to libglib.
Help the static analyser by adding an assertion about the input and
output values for `g_dir_new_from_dirp()`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
It search for attribute trash::orig-path and move the input file to it.
Possibly recreating the directory of orignal path and/or overwritting
the destination.
Closes#2098
Signed-off-by: Frederic Martinsons <frederic.martinsons@sigfox.com>
There were a couple of places where the return value wasn’t checked, and
hence failure could not be noticed.
Coverity CIDs: #1159435, #1159426
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
From:
9eb9c93275
"we found that the const security_context_t declarations in libselinux
are incorrect; const char * was intended, but const security_context_t
translates to char * const and triggers warnings on passing const char *
from the caller. Easiest fix is to replace them all with const char *."
And later marked deprecated in commit:
7a124ca275
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
These variables were already (correctly) accessed atomically. The
`volatile` qualifier doesn’t help with that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
This should introduce no API changes. The
`g_dbus_error_register_error_domain()` function still (incorrectly) has
a `volatile` argument, but dropping that qualifier would be an API
break.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
This should introduce no API changes; there are public functions
exported by `GDBusConnection` which still have some (incorrectly)
`volatile` arguments, but dropping those qualifiers would be an API
break.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
These variables were already (correctly) accessed atomically. The
`volatile` qualifier doesn’t help with that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
And drop the `volatile` qualifier from the variables, as that doesn’t
help with thread safety.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
http://isvolatileusefulwiththreads.in/c/
It’s possible that the variables here are only marked as volatile
because they’re arguments to `g_once_*()`. Those arguments will be
modified in a subsequent commit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
These tests were originally written using the output directly from a
fuzzer which had triggered the bugs we’re testing for. However, that
means they’re liable to no longer test what they’re intended to test if
the `GDBusMessage` parsing code is changed to (for example) check for
certain errors earlier in future.
It’s better to only have one invalidity in each binary blob, so change
the test messages to all be valid apart from the specific thing they’re
testing for.
The changes were based on reading the D-Bus specification directly:
https://dbus.freedesktop.org/doc/dbus-specification.html
During these changes I found one problem in
`test_message_parse_deep_header_nesting()` where it wasn’t actually
nesting variants in the header deeply enough to trigger the bug it was
supposed to be testing for. Fixed that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #1963
This commit is the unmodified results of running
```
black $(git ls-files '*.py')
```
with black version 19.10b0. See #2046.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Set counters for the number of running tasks and
for the max. threadpool size. These are meant to
get a sense for whether G_TASK_POOL_SIZE and related
constants are still suitable for current gio and
GTask usage patterns.
Previously it was considered a programming error to call these on
subprocesses created without the correct flags, but for bindings this
distinction is difficult to handle automatically.
Returning NULL instead does not cause any inconsistent behaviour and
simplifies the API.
As hidden file caches currently work, every look up on a directory caches
its .hidden file contents, and sets a 5s timeout to prune the directory
from the cache.
This creates a problem for usecases like Tracker Miners, which is in the
business of inspecting as many files as possible from as many directories
as possible in the shortest time possible. One timeout is created for each
directory, which possibly means gobbling thousands of entries in the hidden
file cache. This adds as many GSources to the glib worker thread, with the
involved CPU overhead in iterating those in its main context.
To fix this, use a unique timeout that will keep running until the cache
is empty. This will keep the overhead constant with many files/folders
being queried.
Continue to allow overriding the keyring dir, but don’t automatically
create it when running as root.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Coverity CID: #1432485
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
This incidentally also exercises the intended pattern for sending fds in
a D-Bus message: the fd list is meant to contain exactly those fds that
are referenced by a handle (type 'h') in the body of the message, with
numeric handle value n corresponding to g_unix_fd_list_peek_fds(...)[n].
Being able to send and receive file descriptors that are not referenced by
a handle (as in OpenFile here) is a quirk of the GDBus API, and while it's
entirely possible in the wire protocol, other D-Bus implementations like
libdbus and sd-bus typically don't provide APIs that make this possible.
Reproduces: https://gitlab.gnome.org/GNOME/glib/-/issues/2074
Signed-off-by: Simon McVittie <smcv@collabora.com>
In the D-Bus wire protocol, the handle type (G_VARIANT_TYPE_HANDLE, h)
is intended to be an index/pointer into the implementation's closest
equivalent of GUnixFDList: its numeric value has no semantic meaning
(in the same way that the numeric values of pointers have no semantic
meaning), but a handle with value n acts as a reference to the nth fd
in the fd list.
GDBus provides a fairly direct mapping from the wire protocol to the
C API, which makes it technically possible to attach and use fds
without ever referring to them in the message body, and some
GLib-centric D-Bus APIs rely on this.
However, the other major implementations of D-Bus (libdbus and sd-bus)
transparently replace file descriptors with handles when building
messages, and transparently replace handles with file descriptors when
parsing messages. This means they cannot implement D-Bus APIs that do
not follow the conventional meaning of handles as indexes/pointers into
an equivalent of GUnixFDList.
For interoperability, we should encourage D-Bus API designers to follow
the convention, even though code written against GDBus doesn't strictly
need to do so.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Suppose we are sending a 5K message with fds (so data->blob points
to 5K of data, data->blob_size is 5K, and fd_list is non-null), but
the kernel is only accepting up to 4K with each sendmsg().
The first time we get into write_message_continue_writing(),
data->total_written will be 0. We will try to write the entire message,
plus the attached file descriptors; or if the stream doesn't support
fd-passing (not a socket), we need to fail with
"Tried sending a file descriptor on unsupported stream".
Because the kernel didn't accept the entire message, we come back in.
This time, we won't enter the Unix-specific block that involves sending
fds, because now data->total_written is 4K, and it would be wrong to try
to attach the same fds again. However, we also need to avoid failing
with "Tried sending a file descriptor on unsupported stream" in this
case. We just want to write out the data of the rest of the message,
starting from (blob + total_written) (in this exaple, the last 1K).
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2074
Signed-off-by: Simon McVittie <smcv@collabora.com>
This test ensures that g_socket_client_connect_to_host_async() fails if
it is cancelled, but it's not cancelled until after 1 millisecond. Our
CI testers are hitting that race window, and Milan is able to reproduce
the crash locally as well. Switching it from 1ms to 0ms is enough for
Milan to avoid the crash, but not enough for our CI, so let's move the
cancellation to a GSocketClientEvent callback where the timing is
completely deterministic.
Hopefully fixes#2221
g_has_typeof macro is wrongly in the public g_ namespace, internaly
symbols are usually in the glib_ namespace. This will also allow to
define glib_typeof differently on non-GNUC compilers (e.g. c++11
decltype).
This introduces no functional changes, but makes the refcount handling a
little easier to follow by no longer splitting a ref/unref pair across
three callbacks. Now, the ref/unref pairs are all within function-local
scopes.
Coverity CID: #1430783
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
By default, when using g_subprocess_launcher_take_fd() to pass an
FD to a child, the GSubprocessLauncher object also takes ownership
of the FD in the parent, and closes it during finalize(). This is
a reasonable assumption in the majority of the cases, but sometimes
it isn't a good idea.
An example is when creating a GSubprocessLauncher in JavaScript:
here, the destruction process is managed by the Garbage Collector,
which means that those sockets will remain opened for some time
after all the references to the object has been droped. This means
that it could be not possible to detect when the child has closed
that same FD, because in order to make that work, both FDs
instances (the one in the parent and the one in the children) must
be closed. This can be a problem in, as an example, a process that
launches a child that communicates with Wayland using an specific
socket (like when using the new API MetaWaylandClient).
Of course, it isn't a valid solution to manually call close() in
the parent process just after the call to spawn(), because the FD
number could be reused in the time between it is manually closed,
and when the object is destroyed and closes again that FD. If that
happens, it will close an incorrect FD.
One solution could be to call run_dispose() from Javascript on the
GSubprocessLauncher object, to force freeing the resources.
Unfortunately, the current code frees them in the finalize()
method, not in dispose() (this is fixed in !1670 (merged) ) but it
isn't a very elegant solution.
This proposal adds a new method, g_subprocess_launcher_close(),
that allows to close the FDs passed to the child. To avoid problems,
after closing an FD with this method, no more spawns are allowed.
Fix: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1677
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
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.
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.
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.
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.
`g_local_file_fstatat()` needs to fall back to returning an error if
`fstatat()` isn’t defined, which is the case on older versions of macOS
(as well as Windows, which was already handled). Callers shouldn’t call
`g_local_file_fstatat()` in these cases. (That’s already the case.)
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2203
Expose a function that prepares an attribute query string to be passed
to g_file_query_info() to get a list of attributes normally copied with
the file. This function is used by the implementation of
g_file_copy_attributes, and it's useful if one needs to split
g_file_copy_attributes into two stages, for example, when nautilus does
a recursive move of a directory. When files are moved from the source
directory, its modification time changes. To preserve the mtime on the
destination directory, it has to be queried before moving files and set
after doing it, hence these two stages.
Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
The GSubprocessLauncher class lacks a dispose() method, and frees
all their resources in the finalize() method.
This is a problem with Javascript because the sockets passed to a
child process using g_subprocess_launcher_take_fd() aren't closed
in the parent space until the object is fully freed. This means
that if the child closes a socket, it won't be detected until the
GSubprocessLauncher object has been freed by the garbage
collector.
Just closing the socket externally is not a valid solution,
because the finalize() method will close it again, and since
another file/pipe/socket could have been opened in the meantime
and use the same FD number, the finalize() method would close
an incorrect FD.
An example is launching a child process that uses its own
socket for Wayland: the parent creates two sockets with
socketpair(), passes one to the Wayland API (wl_client_create()),
and the other is passed to the child process using
g_subprocess_launcher_take_fd(). But now there are two instances
of that second socket: one in the parent, and another one in the
child process. That means that, if the child closes its socket (or
dies), the Wayland server will not detect that until the
GSubprocessLauncher object is fully destroyed. That means that a
GSubprocessLauncher created in Javascript will last for several
seconds after the child dies, and every window or graphical element
will remain in the screen until the Garbage Collector destroys the
GSubprocessLauncher object.
This patch fixes this by moving the resource free code into a
dispose() method, which can be called from Javascript. This allows
to ensure that any socket passed to the child with
g_subprocess_launcher_take_fd() can be closed even from Javascript
just by calling the method run_dispose().
Fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1670
This combines a massive code re-folding with functionlity expansion
that allows us to track multiple verbs per handler or per application.
Also fixes a few issues and removes a function that made no sense.
Like G_SOURCE_REMOVE and G_SOURCE_CONTINUE, these make it clearer what
it means to return TRUE or FALSE.
In particular, in GDBus methods that fail, the failure case still needs
to return TRUE (unlike the typical GError pattern), leading to comments
like this:
g_dbus_method_invocation_return_error (invocation, ...);
return TRUE; /* handled */
which can now be replaced by:
g_dbus_method_invocation_return_error (invocation, ...);
return G_DUS_METHOD_INVOCATION_HANDLED;
G_DBUS_METHOD_INVOCATION_UNHANDLED is added for symmetry, but is very
rarely (perhaps never?) useful in practice.
Signed-off-by: Simon McVittie <smcv@collabora.com>
The previous parsing code could read off the end of a URI if it had an
incorrect %-escaped character in.
Fix that, and more closely implement parsing for the syntax defined in
RFC 6874, which is the amendment to RFC 3986 which specifies zone ID
syntax.
This requires reworking some network-address tests, which were
previously treating zone IDs incorrectly.
oss-fuzz#23816
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Emptying trash over `gio trash` is a bit slow in comparison to plain
`rm -r`. On my system, it took about 3 min to empty the trash with a
folder containing 600 000 files, which is not ideal as `rm -r` call
took just a few seconds. I found that `g_file_delete` is implemented
differently for locations provided by the trash backend. The trash
backend prevents modifications of trashed content thus the delete
operation is allowed only for the top-level files and folders. So it
is not necessary to recursive delete all files as the permission
denied error is returned anyway. Let's call `g_file_delete` only for
top-level items, which reduces the time necessary for emptying trash
from minutes to seconds...
See: https://gitlab.gnome.org/GNOME/nautilus/-/issues/1589
Some filesystems don't have meaningful access times under at least some
circumstances (see #2189, #2205). In this situation the traditional stat()
and related kernel interfaces have to put something meaningless in the
st_atime field, and have no way to signal that it is meaningless.
However, statx() does have a way to signal that the atime is meaningless:
if the filesystem doesn't provide a useful access time, it will unset
the STATX_ATIME bit (as well as filling in the same meaningless value
for the stx_atime field that stat() would have used, for compatibility).
We don't actually *need* the atime, so never include it in the required
mask. This was already done for one code path in commit 6fc143bb
"gio: Allow no atime from statx" to fix#2189, but other callers were
left unchanged in that commit, and receive the same change here.
It is not actually guaranteed that *any* of the flags in the
returned stx_mask will be set (the only guarantee is that items in
STATX_BASIC_STATS have at least a harmless compatibility value, even if
their corresponding flag is cleared), so it might be better to follow
this up by removing the concept of the required mask entirely. However,
as of Linux 5.8 it looks as though STATX_ATIME is the only flag in
STATX_BASIC_STATS that might be cleared in practice, so this simpler
change fixes the immediate regression.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2205
Signed-off-by: Simon McVittie <smcv@collabora.com>
It is not allowed to be `NULL` or unset if requested by the file
attribute matcher. Derive it from the basename. This doesn’t handle the
situation of a failed UTF-16 to UTF-8 conversion very well, but will at
least return something.
Note that the `g_filename_display_basename()` function can’t be used as
`GWinHttpFile` provides its URI in UTF-16 rather than in the file system
encoding.
This fixes a crash when using GIMP on Windows. Thanks to lillolollo for
in-depth debugging assistance.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #2194
For interoperability with libdbus, we want to use compatible timeouts.
In particular, this fixes a spurious failure of the `gdbus-server-auth`
test caused by libdbus and gdbus choosing to expire the key (cookie) at
different times, as diagnosed by Thiago Macieira. Previously, the libdbus
client would decline to use keys older than 7 minutes, but the GDBus
server would not generate a new key until the old key was 10 minutes old.
For completeness, also adjust the other arbitrary timeouts in the
DBUS_COOKIE_SHA1 mechanism to be the same as in libdbus. To make it
easier to align with libdbus, create internal macros with the same names
and values used in dbus-keyring.c.
* maximum time a key can be in the future due to clock skew between
systems sharing a home directory
- spec says "a reasonable time in the future"
- was 1 day
- now 5 minutes
- MAX_TIME_TRAVEL_SECONDS
* time to generate a new key if the newest is older
- spec says "If no recent keys remain, the server may generate a new
key", but that isn't practical, because in reality we need a grace
period during which an old key will not be used for new authentication
attempts but old authentication attempts can continue (in practice both
libdbus and GDBus implemented this logic)
- was 10 minutes
- now 5 minutes
- NEW_KEY_TIMEOUT_SECONDS
* time to discard old keys
- spec says "the timeout can be fairly short"
- was 15 minutes
- now 7 minutes
- EXPIRE_KEYS_TIMEOUT_SECONDS
* time allowed for a client using an old key to authenticate, before
that key gets deleted
- was at least 5 minutes
- now at least 2 minutes
- at least (EXPIRE_KEYS_TIMEOUT_SECONDS - NEW_KEY_TIMEOUT_SECONDS)
Based on a merge request by Philip Withnall.
Fixes: #2164
Thanks: Philip Withnall
Thanks: Thiago Macieira
Signed-off-by: Simon McVittie <smcv@collabora.com>
This doesn't trigger the cancellation assertion issue when run locally
(the task didn't return yet, so the error is simply overwritten), but
perhaps it ever does in CI. Anyhow, it's good to have a cancellation
test.
After a splice operation is finished, it attempts to 1) close input/output
streams, as per the given flags, and 2) return the operation result (maybe
an error, too).
However, if the operation gets cancelled early and the streams indirectly
closed, the splice operation will try to close both descriptors and return
on the task when both are already closed. The catch here is that getting the
streams closed under its feet is possible, so the completion callback would
find both streams closed after returning on the first close operation and
return the error, but then the second operation could be able to trigger
a second error which would be returned as well.
What happens here is up to further race conditions, if the task didn't
return yet, the returned error will be simply replaced (but the old one not
freed...), if it did already return, it'll result in:
GLib-GIO-FATAL-CRITICAL: g_task_return_error: assertion '!task->ever_returned' failed
Fix this by flagging the close_async() callbacks, and checking that both
close operations did return, instead of checking that both streams are
closed by who knows.
This error triggers a semi-frequent CI failure in tracker, see the summary at
https://gitlab.gnome.org/GNOME/tracker/-/issues/240
statx does not provide stx_atime when querying a file in a read-only
mounted file system. So call to statx should not expect it to be in
the mask. Otherwise we would fail with ERANGE for querying any file in
a read-only file system.
Fixes#2189.
The `make_pollfd()` call can’t fail because it only does so if
`cancellable == NULL`, and we’ve already checked that. Assert that’s the
case, to shut Coverity up and to catch behavioural changes in future.
Coverity CID: #1159433
Signed-off-by: Philip Withnall <withnall@endlessm.com>
