From X2Go-ML-1@baur-itcs.de Sat Jan 14 13:59:51 2017 Received: (at submit) by bugs.x2go.org; 14 Jan 2017 12:59:53 +0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on ymir.das-netzwerkteam.de X-Spam-Level: X-Spam-Status: No, score=-1.9 required=3.0 tests=BAYES_00,URIBL_BLOCKED autolearn=ham version=3.3.2 Received: from localhost (localhost [127.0.0.1]) by ymir.das-netzwerkteam.de (Postfix) with ESMTP id 787E73CDDE for ; Sat, 14 Jan 2017 13:59:51 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at ymir.das-netzwerkteam.de Received: from ymir.das-netzwerkteam.de ([127.0.0.1]) by localhost (ymir.das-netzwerkteam.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JXn9T6p4JKxH for ; Sat, 14 Jan 2017 13:59:32 +0100 (CET) Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.17.24]) by ymir.das-netzwerkteam.de (Postfix) with ESMTPS id 784753CDDD for ; Sat, 14 Jan 2017 13:59:32 +0100 (CET) Received: from [192.168.0.23] ([78.43.90.159]) by mrelayeu.kundenserver.de (mreue101 [212.227.15.145]) with ESMTPSA (Nemesis) id 0Lgpjq-1cp51v28NK-00oEIh; Sat, 14 Jan 2017 13:54:29 +0100 To: submit@bugs.x2go.org Cc: Oleksandr Shneyder , Mike Gabriel From: Stefan Baur Subject: Multiple issues with x2gousbmount Message-ID: Date: Sat, 14 Jan 2017 13:54:16 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="OjDTj6WW8ueV7Ix66iUOrHcvnX5Bn4bmx" X-Provags-ID: V03:K0:o2jxPZKN6wDxEUcWz4AKXnAOjV4yQ7Fvr9obSWSqoGEkN5fKXcS 46HAj5kIx0wGAlXuhlQP43uxSCVYcEfLj5xVXEy7W1vffSzGVQvNofG9uKoL4lBpEC1DZUJ uPKJeI3PB2qYPapoRMCw5wUpuNB+RXL/b88z8hjsa+kDE4UD5XHY65nG5ttkhrX5ied8KUt dQn7P5RbHvyuikWO7cgDA== X-UI-Out-Filterresults: notjunk:1;V01:K0:EkxQqP7FTzo=:gqWe6k2/J2/DMX5HnZi75W oNQHidZ/IEWxSJdNPKMt9CVevb5NFPb6AHgekvLoINrqADXbkMqcp3uctDneXa2GKAO7x/Vb4 YTPOZtUy6uMScHbgmA7gDylUNxGdf7nvlRFlpclipjq/o7ukv8FJtsAsgYjpO82f2npVg9oQK rgalOCwcUaCWRhtFf4nVSkNRjYC14w1A15kZ5j9IPffupFM7SpvW4/4ZBAN1t2/vIdXdbz8KP s2KFd/dEV8wHY9JgqZMalZOFWd8d57ftJFhkNmsLKOeW7ioWQTYBCsOa1f/UfIGAMShSDWIE+ yIwv5+xHO0dLKs7Xgw3ea5jZbeZmrYj2mi+CeMPTYX2k3QQqYBcK0ZA20nI5o3GQE+jtqq3RN oeqLZoY3l2BTTonFUpIr+JdYECfFLRpNcxeXxWBe6gAxE6WnOeOhGgqXbTxDe2wdbaEvh/MYS 30OAPG3ts03+x1DbtJnPz+cc0Z3FbwibV/bchHCUUAFgIWRIcXIE2A8S0hquOBGeOg0tQrNpM qv/El8kE91EDPY2RezUagQQpgkpwzYThlyZyz1XCbF53r3LeUM144GvIibToWZi3RsLh+XPCx gaZnFKYvc3ZZFu0d43Pf1eBWrJoQXAkulbQ12HChWJ4ujkc2T5pi1j0LjvfLZJS+sei/4mN7t UEWXBYEdK+4eMesvVesGeFRUWG6qDlG4Zns4TdUMTrPYohUyMa2osE0acVROKkozDmLA= This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --OjDTj6WW8ueV7Ix66iUOrHcvnX5Bn4bmx Content-Type: multipart/mixed; boundary="F7KNbDfmHBh83UBg91I6IMhJmsAk3fLDC"; protected-headers="v1" From: Stefan Baur To: submit@bugs.x2go.org Cc: Oleksandr Shneyder , Mike Gabriel Message-ID: Subject: Multiple issues with x2gousbmount --F7KNbDfmHBh83UBg91I6IMhJmsAk3fLDC Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable package: x2gousbmount Hi $LIST, hi Alex, hi Mike#1, due to a recent bug report on X2Go-User, I needed to take a closer look at the x2gousbmount package, and the script /usr/lib/x2go/tce/x2gousbmount in it. I'll be referencing http://code.x2go.org/gitweb?p=3Dx2gothinclient.git;a=3Dblob;f=3Dusbmount/= x2gousbmount;h=3D4da1fbd90b14dda1beb230ea4b08441a77d9e4eb;hb=3D75211b6e72= 4bd896d85d915c754703cabcf37187 for line numbers below, even though that version is more advanced as the one in the stable x2gousbmount package. I'm hoping that Alex or someone else that has a clue wrt/ Perl and this script can shed some light on these issues. Mike#1, you're in CC due to issue #3, which seems to have been introduced in your commit. Feel free to suggest I should turn every issue into a separate bug report - I'm not filing them separately at the moment because the issues seem all more or less interconnected, so I figured I should start with one bug report for "the big picture". Issue #1: Lines 88 and 138 contain a call ... expand_filename("~x2gothinclient ... Now, I understand that this is an attempt to determine the path to the home directory of the user named "x2gothinclient". Which, in the classic X2Go-TCE, is "/var/lib/x2gothinclient". So far, so good. But: Lines 107 and 148 contain what looks to me like a subtle typo: ... expand_filename("~/x2gothinclient ... Which means, instead of selecting the home directory of the user x2gothinclient, it would go to the home directory of the user under which the script is being executed, and a subdirectory "x2gothinclient" there, so most likely /root/x2gothinclient (which doesn't exist) So the script will probably fail whenever it encounters encrypted volumes= =2E (I can't test this myself, I don't have a setup with encrypted USB media.= ) Issue #2: The user name "x2gothinclient" is hardcoded in several places inside the script. This calls at least for a "my $user=3D'x2gothinclient';" at the beginning= of the script, and the replacement of every occurrence of "x2gothinclient" with "$user". The reason being that TCE-Live uses a different user name, namely "user" (and a different home directory, /home/user), so we need to be able to swap out the name in one central location. There are two ways of determining that we're running in TCE-Live, I am not sure which one would be the preferable one: 1) Checking /proc/cmdline for the appearance of (in regex syntax) "\W*live\W*" or "\W*boot=3Dlive\W*" (in case your mail software turns asterisks into bold type, there's an asterisk after each capital W). 2) Checking if the directory /lib/live or /lib/live/config exists. If any of these match, then $user should be set to 'user' instead of 'x2gothinclient'. Issue #3: Comparing the master branch in git with the version in stable, line 37 is different. There now is an additional check for the existence of a directory "/usr/share/doc/x2gothinclient-minidesktop". Which script/package creates this directory and why is it equivalent to detecting a running x2gothinclientd? Looks like http://code.x2go.org/gitweb?p=3Dx2gothinclient.git;a=3Dcommit;h=3Da458a1e= d666eb12b5d856812396ecd31fba01585 introduced that change, see http://code.x2go.org/gitweb?p=3Dx2gothinclient.git;a=3Dcommit;h=3Da458a1e= d666eb12b5d856812396ecd31fba01585 but there's no meaningful commit message. Mike#1? Issue #4: I am unhappy with the subroutine check_x2gothinclientd. Indeed, grepping for substrings so you don't trigger on the parameters of your own grep command in the output of ps is a neat hack, but a hack remains a hack. The clean way of handling this, IMO, would be to change the subroutine as follows: sub check_x2gothinclientmode { # check if X2GoClient is running in thinclient mode # old code would check if x2gothinclientd was running, # which fails on X2Go-TCE-live my $x=3D`ps u -C x2goclient`; if ( $x=3D~m/\W*--thinclient\W*/ ) { return 1; } return 0; } This will no longer detect if x2gothinclientd is running, but if x2goclient has been called with parameter --thinclient. Which should be the case in both TCE-Classic and TCE-Live. Due to the name change of the subroutine, line 37 needs to be changed to use check_x2gothinclientmode instead of check_x2gothinclientd. Maybe the changed check means the || - part in line 37 is no longer needed as well? Issue #5: Lines 88 and 138 silently assume that there is a subdirectory "export". I can't see it being created anywhere, though. Same goes for lines 107 and 148 and the subdirectory "logins". Issue #6: Why do we need two separate subdirectories "export" and "logins", anyways= ? Issue #7: Somewhere around line 48, I'm missing a comment that explains what this part of the code is for. I would suggest adding: # mntdir is not the directory where the mountpoint will be rooted, # but where tracking of mount states takes place Issue #8: Why do we have to track these mountpoints manually, anyways? Is the information in /proc/mounts insufficient? -Stefan --=20 BAUR-ITCS UG (haftungsbeschr=C3=A4nkt) Gesch=C3=A4ftsf=C3=BChrer: Stefan Baur Eichen=C3=A4ckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364 Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243 --F7KNbDfmHBh83UBg91I6IMhJmsAk3fLDC-- --OjDTj6WW8ueV7Ix66iUOrHcvnX5Bn4bmx Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJYeh97AAoJEG7d9BjNvlEZNE4IAKzB5ZKpxZObcz3u8f4a8gBp ZpE/kQTGCtltYjcDHt7Embg1KQU7F+lTWO1MiaykNLsQguuAagtV+X5bwT+hC2Mt lBJKRXW2dBzNnYqldkGsvyp3+SopqMNMCV6rEKaBpFANxj2pnz72aW7aZwMPtHRq X7VxQUvWcDxvpaBOE9aTzkKqhijk4lHk7OpQj503iLYYSNRFXE14zl2mix1e/WfO s/ie8/Z3s8vvqB94/yjXI2M482Sv+3JvtbmedCd22Ee88nxvoT0TBa5tJWKeLcCF /DqEN4psm2J+l+/Oqh4/Z5HM1sWSy5IvKqFVCYatnbBVzy5i7t35O3s2R09pEO0= =M4U2 -----END PGP SIGNATURE----- --OjDTj6WW8ueV7Ix66iUOrHcvnX5Bn4bmx--