Created attachment 56874 [details] [review] Remove duplicate nonce-tcp (client side) transport on Windows _dbus_transport_open_socket is called before _dbus_transport_open_platform_specific, and now handles nonce-tcp, so this version is no longer useful. --- I noticed this while looking at another bug. I don't run Windows, so this will need testing, but I can confirm that nonce-tcp does work on Unix...
Created attachment 56875 [details] [review] Remove duplicate nonce-tcp (service-side) transport on Windows Turns out this was duplicated too. We can just use the platform-independent version, which uses the same code.
(In reply to comment #0) > Created attachment 56874 [details] [review] [review] > Remove duplicate nonce-tcp (client side) transport on Windows > > _dbus_transport_open_socket is called before > _dbus_transport_open_platform_specific, and now handles nonce-tcp, so > this version is no longer useful. > > --- > > I noticed this while looking at another bug. I don't run Windows, so this will need testing, but I can confirm that nonce-tcp does work on Unix... Which kind of nonce-tcp address do you have used, so that this can be verified on windows ?
(In reply to comment #2) > > I noticed this while looking at another bug. I don't run Windows, > > so this will need testing, but I can confirm that nonce-tcp does work > > on Unix... > > Which kind of nonce-tcp address do you have used, so that this can be > verified on windows ? The verification I'm asking for is more like "with these patches applied, normal usage on Windows still works". I believe it should, because there's now a second copy of nonce-tcp in dbus-transport-socket, and the one in dbus-transport-socket gets tried first, so as far as I can see, the Windows-specific copy is never actually run (even on Windows). What I've tested on Unix is test/loopback.c, which puts a DBusServer on "nonce-tcp:host=127.0.0.1" then connects to whatever port the DBusServer ends up with.
(In reply to comment #3) > (In reply to comment #2) > > > I noticed this while looking at another bug. I don't run Windows, > > > so this will need testing, but I can confirm that nonce-tcp does work > > > on Unix... > > > > Which kind of nonce-tcp address do you have used, so that this can be > > verified on windows ? > > The verification I'm asking for is more like "with these patches applied, normal usage on Windows still works". I believe it should, because there's now a > second copy of nonce-tcp in dbus-transport-socket, and the one in dbus-transport-socket gets tried first, so as far as I can see, the Windows-specific copy is never actually run (even on Windows). > > What I've tested on Unix is test/loopback.c, unfortunally this test depends on glib, which is not integrated into windows windows dbus, so I cannot run it. > which puts a DBusServer on "nonce-tcp:host=127.0.0.1" then connects to whatever port the DBusServer ends up with. As a workaround I used "nonce-tcp:host=127.0.0.1" in session.conf and launched a dbus-daemon with that address. The log now reported me the real used session bus address nonce-tcp:host=127.0.0.1,port=39373,noncefile=C%3a\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n,guid=83380282fd7a433021c38fc14f424648 which I used to set the DBUS_SESSION_BUS_ADDRESS environment variable. Now I can connect to the dbus-daemon using qdbusviewer. Then i killed dbus-daemon and used the following address in session.conf nonce-tcp:host=127.0.0.1,port=39373,noncefile=C%3a\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n,guid=83380282fd7a433021c38fc14f424648 and set DBUS_SESSION_BUS_ADDRESS environment to nonce-tcp:host=127.0.0.1,port=39373,noncefile=C%3a\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n,guid=83380282fd7a433021c38fc14f424648 and run qdbusviewer to which i *cannot* connect to. I found the following message in the log [3928] qdbusviewer: [dbus\dbus-transport.c(205):_dbus_transport_init_base] Initialized transport on address nonce-tcp:host=127.0.0.1,port=39373noncefile=C:\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n There is a ',' missing between the port value and the 'noncefile' string port=39373noncefile There seems to be something broken in the nonce-tcp stuff.
(In reply to comment #4) > There seems to be something broken in the nonce-tcp stuff. Indeed. Do you get the same failure without the patches from this bug? (If so, it's not a regression, so perhaps we should merge them anyway, to have less duplicate code.)
(In reply to comment #4) > unfortunally this test depends on glib, which is not integrated into windows > windows dbus, so I cannot run it. FYI, it's not integrated into cmake, but works fine in Autotools-targeting-Windows (I have a cross-compilation environment using mingw-w64). > The log now reported me the real used session bus address > > nonce-tcp:host=127.0.0.1,port=39373,noncefile=C%3a\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n,guid=83380282fd7a433021c38fc14f424648 > > which I used to set the DBUS_SESSION_BUS_ADDRESS environment variable. This is almost what loopback.c does (but using dbus-daemon --print-address rather than a debug log). Bug #46049 prevents that from working on Windows. > Then i killed dbus-daemon and used the following address in session.conf > > nonce-tcp:host=127.0.0.1,port=39373,noncefile=C%3a\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n,guid=83380282fd7a433021c38fc14f424648 It doesn't usually make sense to force a specific port/noncefile/guid in the listening address (what you did first was a more realistic use of nonce-tcp), but OK... > [3928] qdbusviewer: [dbus\dbus-transport.c(205):_dbus_transport_init_base] > Initialized transport on address > nonce-tcp:host=127.0.0.1,port=39373noncefile=C:\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n > > There is a ',' missing between the port value and the 'noncefile' string > port=39373noncefile This is a bug in platform-independent code (dbus-transport-socket.c); it's equally broken on Unix and Windows. Patch on the way, but it's pretty obvious (add some commas).
(In reply to comment #4) > Then i killed dbus-daemon and used the following address in session.conf > > nonce-tcp:host=127.0.0.1,port=39373,noncefile=C%3a\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n,guid=83380282fd7a433021c38fc14f424648 That's not meant to work: the server (dbus-daemon in this case) always ignores the 'noncefile' and 'guid' address entries, and generates new ones. The Windows-specific duplicate code that I deleted from dbus/dbus-server-win.c in Attachment #56875 [details] doesn't read those entries either. (I'm not trying to change how anything works here, just remove duplication: there's nothing Windows-specific about nonce-tcp, and as far as I can see, the code structure is such that the generic copy is already the only one that gets used.)
Created attachment 57582 [details] [review] _dbus_transport_new_for_tcp_socket: add missing commas to address Ralf pointed out that the address doesn't round-trip correctly. --- This is actually just cosmetic, as far as I can see: it's in the client side (transport, not server), so nothing except debug messages should be using this value. Still, it's good to fix it.
(In reply to comment #7) > (In reply to comment #4) > > Then i killed dbus-daemon and used the following address in session.conf > > > > nonce-tcp:host=127.0.0.1,port=39373,noncefile=C%3a\Users\admin\AppData\Local\Temp/dbus_nonce-FHI1Sq4n,guid=83380282fd7a433021c38fc14f424648 > > That's not meant to work: the server (dbus-daemon in this case) always ignores > the 'noncefile' and 'guid' address entries, and generates new ones. The > Windows-specific duplicate code that I deleted from dbus/dbus-server-win.c in > Attachment #56875 [details] doesn't read those entries either. > > (I'm not trying to change how anything works here, just remove duplication: > there's nothing Windows-specific about nonce-tcp, and as far as I can see, the > code structure is such that the generic copy is already the only one that gets > used.) sure, I tried to get the nonce-tcp stuff runnning on windows, but failed because I did not find the correct way how to use it. When I use 'nonce-tcp:host=127.0.0.1' as listening address, I wasn't able to figure out which related client address to use, especially how the client will get the name for the noncefile file. I tried some more combinations and had only success with the above mentioned address.
(In reply to comment #9) > When I use 'nonce-tcp:host=127.0.0.1' as listening address, I wasn't able to > figure out which related client address to use, especially how the client will > get the name for the noncefile file. In C code, you use dbus_server_get_address() to get the client address out (see loopback.c). When running dbus-daemon, you use dbus-daemon --print-address, except that doesn't work on Windows, because of Bug #46049. With that bug fixed (I attached a patch, but I've only tested it under emulation), you can get the address out that way, and use it as the client address. Either of those should give you the same address that was in the debug log; you can put it in DBUS_SESSION_BUS_ADDRESS to test it, for instance. (That's all that's meant to work; there's nothing as elaborate as Windows autolaunch.) In the normal Unix setup, dbus-launch puts the address from dbus-daemon --print-address in the DBUS_SESSION_BUS_ADDRESS environment variable and a hidden X11 window when you log in (or just in the hidden X11 window, when autolaunching occurs), and everything else gets it from there.
(In reply to comment #10) > (In reply to comment #9) > > When I use 'nonce-tcp:host=127.0.0.1' as listening address, I wasn't able to > > figure out which related client address to use, especially how the client will > > get the name for the noncefile file. > > In C code, you use dbus_server_get_address() to get the client address out (see > loopback.c). > > When running dbus-daemon, you use dbus-daemon --print-address, except that > doesn't work on Windows, because of Bug #46049. With that bug fixed (I attached > a patch, but I've only tested it under emulation), you can get the address out > that way, and use it as the client address. > > Either of those should give you the same address that was in the debug log; you > can put it in DBUS_SESSION_BUS_ADDRESS to test it, for instance. (That's all > that's meant to work; there's nothing as elaborate as Windows autolaunch.) > > In the normal Unix setup, dbus-launch puts the address from dbus-daemon > --print-address in the DBUS_SESSION_BUS_ADDRESS environment variable and a > hidden X11 window when you log in (or just in the hidden X11 window, when > autolaunching occurs), and everything else gets it from there. After fixing #46049 nonce-tcp protocol will still not be usable on windows because 1. dbus-launch on windows will not fetch the address 2. dbus client library still uses CreateProcess to spawn dbus-daemon on autolaunching 3. dbus-launch or dbus-client library do not store the session bus address into the parent environment Setting parent environment on windows looks complicated http://www.perlmonks.org/?node_id=658278 and registry settings are unusable for portable installations, so here we are in trouble
(In reply to comment #11) > After fixing #46049 nonce-tcp protocol will still not be usable on windows I don't mind that it doesn't work without configuration; neither do the tcp and unix transports, or the nonce-tcp transport on Unix. All I want is that if you have an out-of-band way to communicate the server address (i.e. dbus_server_get_address()) to the client library, nonce-tcp works. Your testing has already demonstrated that that's true (Comment #4), as far as I can see. What I'm mainly trying to achieve with this bug is just to get rid of a duplicate secondary copy of part of the nonce-tcp implementation. In Unix builds, there is only one copy of the setup logic for nonce-tcp, in dbus-transport-socket.c (for clients) and dbus-server-socket.c (for servers). I know that it works on Unix, because loopback.c tests it; it didn't work on Unix before commit 54de9c060 due to Bug #34569. On Windows, there is the same code as on Unix, and also a second copy in the Windows-specific files, which is deleted by Attachment #56874 [details] and Attachment #56875 [details]. As far as I can tell, the platform-independent version is the one that is actually used for nonce-tcp addresses, because it comes first in the search order, so the second, Windows-specific version is never reached. I assume that the Windows-specific version was written first, and then someone noticed that there was no reason why it couldn't work on Unix too, and copied it into the generic transport code. I don't know why they copied it instead of moving it; it seems to have been duplicated ever since nonce-tcp was first added to the main D-Bus repository (commit 5e2a99c12c7e3). If you've reviewed the patches attached here and they are OK, please say so and I'll merge them :-) > Setting parent environment on windows looks complicated > http://www.perlmonks.org/?node_id=658278 You can't do that on Unix either (the situation is the same, except that there is no Registry, so you can't use that method even if there weren't reasons not to do so). So, no problem there.
(In reply to comment #12) > If you've reviewed the patches attached here and they are OK, please say so and > I'll merge them :-) > Found some time to review this. nonce-tcp works with this patches on windows.
(In reply to comment #13) > Found some time to review this. nonce-tcp works with this patches on windows. Thanks, fixed in git for 1.6.0.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.