Introduce and use EscapeBusAddressValue#302
Merged
guelfey merged 1 commit intogodbus:masterfrom Feb 13, 2022
Merged
Conversation
kolyshkin
added a commit
to kolyshkin/runc
that referenced
this pull request
Feb 1, 2022
D-Bus specification [1] requires that the values in server address need to be escaped in a special way, and other clients perform the needed escaping (e.g. systemd [2] does that). More to say, if dbus sees a character that should have been escaped, it returns an error [3]. Add escapeBusAddressValue function (with some tests and a benchmark), and use it from tryDiscoverDbusSessionBusAddress(). NOTE that the function does not escape '*' symbol, because de-facto it is the list of optionally-escaped characters since 2007, but that is not yet documented in D-Bus spec (see [4] for more details). TODO: switch to dbus.EscapeBusAddressValue if/when godbus/dbus#302 is merged. [1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses [2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318 [3] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330 [5] https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/248 Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
force-pushed
the
escape-bus-address
branch
2 times, most recently
from
47b6dc1 to
1c520fd
Compare
guelfey
approved these changes
Feb 5, 2022
Member
guelfey
left a comment
guelfey
left a comment
There was a problem hiding this comment.
Makes sense. Shouldn't we then consequently unescape the address in e.g. Dial?
Contributor
Author
|
Well, it seems that |
D-Bus specification [1] requires that the values in server address need to be escaped in a special way, and other clients perform the needed escaping (e.g. systemd [2] does that). More to say, if dbus sees a character that should have been escaped, it returns an error [3]. Add EscapeBusAddressValue function (with some tests and a benchmark), and use it from tryDiscoverDbusSessionBusAddress(). Add UnescapeBusAddressValue, and use it from getKey(). The functions are exported as they might be of use to other packages (in particular, runc [4]). NOTE that the Escape... function does not escape '*' symbol, because de-facto it is the list of optionally-escaped characters since 2007, but that is not yet documented in D-Bus spec (see [5] for more details). [1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses [2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318 [3] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330 [4] opencontainers/runc#3356 [5] https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/248 Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
force-pushed
the
escape-bus-address
branch
from
1c520fd to
48d30c5
Compare
Contributor
Author
|
Added UnescapeBusAddressValue, and use it from |
guelfey
approved these changes
Feb 13, 2022
Member
|
Thanks! |
Contributor
Author
|
@guelfey is it possible to have a release with this feature included? |
kolyshkin
added a commit
to kolyshkin/runc
that referenced
this pull request
Feb 16, 2022
This is mostly to include godbus/dbus#302. Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
added a commit
to kolyshkin/runc
that referenced
this pull request
Feb 16, 2022
D-Bus specification [1] requires that the values in server address need to be escaped in a special way, and other clients perform the needed escaping (e.g. systemd [2] does that). More to say, if dbus sees a character that should have been escaped, it returns an error [3]. Fix tryDiscoverDbusSessionBusAddress to use dbus.EscapeBusAddressValue function, recently added to godbus [4]. [1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses [2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318 [3] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330 [4] godbus/dbus#302 Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
added a commit
to kolyshkin/runc
that referenced
this pull request
Feb 16, 2022
D-Bus specification [1] requires that the values in server address need to be escaped in a special way, and other clients perform the needed escaping (e.g. systemd [2] does that, as well as recent godbus [3]). More to say, it is important to perform such escaping, since if dbus sees a character that should have been escaped but it's not, it returns an error [4]. Fix tryDiscoverDbusSessionBusAddress to use dbus.EscapeBusAddressValue function, recently added to godbus [3]. [1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses [2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318 [3] godbus/dbus#302 [4] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330 Signed-off-by: Kir Kolyshkin <[email protected]>
Member
|
Done in https://github.com/godbus/dbus/releases/tag/v5.1.0 - bumped the minor version to reflect the added APIs |
kolyshkin
added a commit
to kolyshkin/runc
that referenced
this pull request
Mar 1, 2022
D-Bus specification [1] requires that the values in server address need to be escaped in a special way, and other clients perform the needed escaping (e.g. systemd [2] does that, as well as recent godbus [3]). More to say, it is important to perform such escaping, since if dbus sees a character that should have been escaped but it's not, it returns an error [4]. Fix tryDiscoverDbusSessionBusAddress to use dbus.EscapeBusAddressValue function, recently added to godbus [3]. [1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses [2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318 [3] godbus/dbus#302 [4] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330 Signed-off-by: Kir Kolyshkin <[email protected]>
dims
pushed a commit
to dims/libcontainer
that referenced
this pull request
Oct 19, 2024
D-Bus specification [1] requires that the values in server address need to be escaped in a special way, and other clients perform the needed escaping (e.g. systemd [2] does that, as well as recent godbus [3]). More to say, it is important to perform such escaping, since if dbus sees a character that should have been escaped but it's not, it returns an error [4]. Fix tryDiscoverDbusSessionBusAddress to use dbus.EscapeBusAddressValue function, recently added to godbus [3]. [1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses [2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318 [3] godbus/dbus#302 [4] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330 Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
added a commit
to kolyshkin/containerd-cgroups
that referenced
this pull request
Nov 6, 2024
D-Bus specification [1] requires that the values in server address need to be escaped in a special way, and other clients perform the needed escaping (e.g. systemd [2] does that, as well as recent godbus [3]). More to say, it is important to perform such escaping, since if dbus sees a character that should have been escaped but it's not, it returns an error [4]. Fix tryDiscoverDbusSessionBusAddress to use dbus.EscapeBusAddressValue function, recently added to godbus [3]. [1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses [2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318 [3] godbus/dbus#302 [4] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330 Signed-off-by: Kir Kolyshkin <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
D-Bus specification [1] requires that the values in server address need
to be escaped in a special way, and other clients perform the needed
escaping (e.g. systemd [2] does that).
More to say, if dbus sees a character that should have been escaped, it
returns an error [3].
Add EscapeBusAddressValue function (with some tests and a benchmark),
and use it from tryDiscoverDbusSessionBusAddress().
The function is exported as it might be of use to other packages (in
particular, runc [4]).
NOTE that the function does not escape '*' symbol, because de-facto it
is the list of optionally-escaped characters since 2007, but that is not
yet documented in D-Bus spec (see [5] for more details).
[1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses
[2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318
[3] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330
[4] opencontainers/runc#3356
[5] https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/248