-
-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various development questions #3276
Comments
From Xpra manual I understood that the socket is expected to be created to either all places specified by However, if If it is an intended behaviour, then manpage must be clarified. If it is a bug, the fix is trivial but I don't want to push it as PR without clear understanding of your intent first.
but I encountered another bunch of logic flaws. First issue is that https://github.com/Xpra-org/xpra/blob/master/xpra/server/auth/sys_auth_base.py#L67 So does The result is that no sockets can be found in Second issue is that https://github.com/Xpra-org/xpra/blob/master/xpra/scripts/main.py#L1191 despite diff -ru --color /tmp/xpra/xpra/scripts/main.py /usr/lib/python3/dist-packages/xpra/scripts/main.py
--- /tmp/xpra/xpra/scripts/main.py 2021-09-21 10:29:37.875777899 +0300
+++ /usr/lib/python3/dist-packages/xpra/scripts/main.py 2021-09-21 18:52:34.442602993 +0300
@@ -1188,11 +1188,12 @@
sockpath = display_desc.get("socket_path")
if not sockpath:
#find the socket using the display:
+ uid = display_desc.get("uid", 0)
dotxpra = DotXpra(
display_desc.get("socket_dir"),
display_desc.get("socket_dirs"),
- display_desc.get("username", ""),
- display_desc.get("uid", 0),
+ display_desc.get("username", get_username_for_uid(uid)),
+ uid,
display_desc.get("gid", 0),
)
display = display_desc["display"] and if For industrial adoption of this scenario, we need either a separate option like |
Yes.
Where is the part that is confusing or unclear? As for the rest, I will take a look tomorrow when my eyes line up better with my eye sockets! |
if the connection options don't override it
The commit above should fix that (untested). But specifying it as part of the socket is not a bad way of doing things. The only problem is that you can only specify a single value in
Your change looks good. do you want to send a PR or do you want me to just commit it?
Isn't that root in this case?
I'm lost now..
What is "foreign sockets destination"?
I'm really not keen on a full recursive search. And the directory But initially, sockets would be created in both locations in |
Close but no cigar :)
where AFAIK the former solution worked because
Let me think of it. Currently grouping is quite intuitive.
I will PR it with fallback for
It can be caused by either missing username or uid/gid/username that has no local equivalent, so we need a fallback on "proxy" side
I call the directory that is searchable by
Perfect! You got my point!!! |
so the authentication module can expand it with the username / uid / gid that it wants to use
You were faster than I editing the comment above in re unexpanded set of values :) |
The commit above should give you
Cool. I'll have a go at it as soon as I find the time. |
Nope :(
I added it as the last |
@basilgello can you give me the simplest command line possible to reproduce? |
|
From https://github.com/Xpra-org/xpra/blob/master/fs/share/man/man1/xpra.1#L502
This reads like Unix domain socket will be created only in |
@totaam If command-line options like
|
Yes, it overrides. Otherwise there would be no way to remove values. ie |
OK, so now the only thing to check why paths not get expanded in authenticators :) Hm...
printing |
@totaam There was an easy fix: https://github.com/Xpra-org/xpra/blob/master/xpra/server/auth/sys_auth_base.py#L217
|
Good catch! |
I refactored the patch as follows: diff -ru --color /tmp/xpra/xpra/scripts/main.py /usr/lib/python3/dist-packages/xpra/scripts/main.py
--- /tmp/xpra/xpra/scripts/main.py 2021-09-23 14:20:56.464094928 +0300
+++ /usr/lib/python3/dist-packages/xpra/scripts/main.py 2021-09-23 14:19:22.342702599 +0300
@@ -1188,12 +1188,21 @@
sockpath = display_desc.get("socket_path")
if not sockpath:
#find the socket using the display:
+ # if uid, gid or username are missing or not found on
+ # local system, use uid, gid, username of authenticated user
+ uid = display_desc.get("uid", getuid())
+ gid = display_desc.get("gid", getgid())
+ username = display_desc.get("username", get_username_for_uid(uid))
+ if not username:
+ uid = getuid()
+ gid = getgid()
+ username = get_username_for_uid(uid)
dotxpra = DotXpra(
display_desc.get("socket_dir"),
display_desc.get("socket_dirs"),
- display_desc.get("username", ""),
- display_desc.get("uid", 0),
- display_desc.get("gid", 0),
+ username,
+ uid,
+ gid,
)
display = display_desc["display"]
def socket_details(state=DotXpra.LIVE): so that if there is no If you see no caveats, just commit it :) |
Also, again we have an issue with Firefox in HTML5 client:
You asked me in Xpra-org/xpra-html5#92 if it is related to image file itself. No, it is a Firefox issue that will be likely patched in FF93: https://bugzilla.mozilla.org/show_bug.cgi?id=1533680 and mrdoob/three.js#21003 Till that time, I think we'll need to pass a minimal image to |
I'm seeing this too! (I was certain I had tested with Firefox but perhaps not with png or jpeg encoding) - fixed: |
Also another minor 🐛 in HTML5: despite the Menu position is |
|
if the connection options don't override it
so the authentication module can expand it with the username / uid / gid that it wants to use
I can see the email in the smtp log, but not the email in my mailbox!
That comment link is broken. Can you edit it?
Sounds reasonable and I can't find a valid reason against it. (though I am feeling slightly uncomfortable - like I am missing something obvious) |
Done
Done
Will do :) |
The only thing that looks like something buzzing from the back is that you will need to propagate #3281 to 4.2.x and 4.1.x, where you backported "use socket-dirs from servers in authenticators". 3.x is likely not affected by regression. |
v4.2.x: ef6688f @basilgello Why would 3.x not be affected? It has 6a464c4 |
Then 3.x is affected, too! |
v3.1.x: b0f14d3 I think we can close this ticket? |
Uhm, I created this ticket like a chat to discuss questions that don't deserve a separate ticket :) Those who deserve one will be spawned into separate issues. Or you feel uncomfortable with such "chatty" issue? |
I don't mind. I just like to close tickets as fast as I can! |
"Zarro Boogs Encountered" :D |
Meh...
and all subsequent directory operations spill errors. Source is likely this commit: e41a801 Solution is to create proto-directory, try moving it and if the smoke test fails, try next destination, or just create directory where it should be moved. |
This is just fallout from #3217. The commit you pointed to is actually supposed to make it less likely that we'll choose a different session directory once we have the actual display number, by re-using the same function to get the directory path. I don't really understand how you got different paths here. The first call to: session_dir = make_session_dir(opts.sessions_dir, display_name, uid, gid) Should have also used We cannot move files across filesystems.. and copying then removing is tricky / dangerous. Especially as root. |
Let me investigate. The issue appears only when Xpra backend server is started by Xpra proxy server. |
First call on
At this time probably Second call to
Fix: https://github.com/Xpra-org/xpra/blob/master/xpra/scripts/server.py#L364 session_dir = get_session_dir(sessions_dir, display_name, uid) |
#3284 fixes a data race in XDG menu sending from Xpra backend server to clients. |
Did you notice the fix above for directory paths? :) |
Where? |
|
Oh, so it's just the uid missing. |
I see you started doing that closing #3217 right? Ping me please when you need a test build. Also, I think it is perfect time to convince Dmitry to upload 4.x to Debian |
xpra main error: жов 15 00:01:14 buildbot xpra[1960395]: Traceback (most recent call last): жов 15 00:01:14 buildbot xpra[1960395]: File "/usr/lib/python3/dist-packages/xpra/scripts/main.py", line 140, in main жов 15 00:01:14 buildbot xpra[1960395]: return run_mode(script_file, cmdline, err, options, args, mode, defaults) жов 15 00:01:14 buildbot xpra[1960395]: File "/usr/lib/python3/dist-packages/xpra/scripts/main.py", line 413, in run_mode жов 15 00:01:14 buildbot xpra[1960395]: return do_run_mode(script_file, cmdline, error_cb, options, args, mode, defaults) жов 15 00:01:14 buildbot xpra[1960395]: File "/usr/lib/python3/dist-packages/xpra/scripts/main.py", line 443, in do_run_mode жов 15 00:01:14 buildbot xpra[1960395]: return run_server(script_file, cmdline, error_cb, options, args, mode, defaults) жов 15 00:01:14 buildbot xpra[1960395]: File "/usr/lib/python3/dist-packages/xpra/scripts/main.py", line 1743, in run_server жов 15 00:01:14 buildbot xpra[1960395]: return do_run_server(script_file, cmdline, error_cb, options, args, mode, display, defaults) жов 15 00:01:14 buildbot xpra[1960395]: File "/usr/lib/python3/dist-packages/xpra/scripts/server.py", line 651, in do_run_server
жов 15 00:01:14 buildbot xpra[1960395]: return _do_run_server(script_file, cmdline, жов 15 00:01:14 buildbot xpra[1960395]: File "/usr/lib/python3/dist-packages/xpra/scripts/server.py", line 887, in _do_run_server
жов 15 00:01:14 buildbot xpra[1960395]: if x=="/run/user/%i" % uid:
|
OK, so Now two unrelated questions. First: how does one list and switch between windows ( Second: I am planning to add not only proper keyboard to HTML5 client, but also mouse buttons to have long press of right/center button/scroll. What do you think on it? |
Oops. I had not tested the proxy as root... because I knew you would! (fixed in 1a0adc6)
The top bar is one way.
That would be awesome!
I'm not sure what's missing there, can you clarify? "long press"? |
Oops I thought it is screenshot maker!!!
Splashtop Android client has an option to spawn left, right, center button and scroll into a separate pane that can be placed for example on right border of screen. The virtual mouse buttons can "stick" emulating drag with mouse button pressed. Also it is super useful for right clicks :) Ideally, trackball mode is mandatory to have, too. CAD app users will say thank you to us ;) |
This ticket is becoming too hard to follow, please open new issues instead. |
@totaam Let's keep this issue for various ongoing questions which don't deserve the seaprate issue instead of using email.
The text was updated successfully, but these errors were encountered: