Bug 220668 - x11/xinit: Incorrectly calculates $displayname used by xauth
Summary: x11/xinit: Incorrectly calculates $displayname used by xauth
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Matthew Rezny
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2017-07-12 06:00 UTC by Duane
Modified: 2017-08-27 19:29 UTC (History)
3 users (show)

See Also:
rezny: maintainer-feedback+
koobs: merge-quarterly?


Attachments
Patch to remove warnings and align displayname with `xauth` (1.05 KB, patch)
2017-07-12 23:16 UTC, Duane
no flags Details | Diff
Additionally fix `expr` bashism (1.25 KB, patch)
2017-07-18 02:11 UTC, Duane
no flags Details | Diff
fixup x11/xinit (2.91 KB, patch)
2017-07-30 15:01 UTC, Matthew Rezny
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Duane 2017-07-12 06:00:19 UTC
`/usr/local/bin/startx` does not correctly calculate the `$displayname` value used by `xauth`.  Specifically the `xauth` man page states that same-machine connections should use the form `$hostname/unix:$display`.

The following patch should fix the behaviour.

@@ -192,9 +193,9 @@
     # now add the same credentials to the client authority file
     # if '$displayname' already exists do not overwrite it as another
     # server man need it. Add them to the '$xserverauthfile' instead.
-    for displayname in $authdisplay $hostname$authdisplay; do
+    for displayname in $authdisplay $hostname/unix$authdisplay; do
         authcookie=`xauth list "$displayname" \
-        | sed -n "s/.*$displayname[[:space:]*].*[[:space:]*]//p"` 2>/dev/null;
+            | sed -n "s|.*$displayname[[:space:]*].*[[:space:]*]||p"` 2>/dev/null;
         if [ "z${authcookie}" = "z" ] ; then
             xauth -q << EOF 
 add $displayname . $mcookie
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2017-07-12 07:33:52 UTC
Thank you for your report and patch Duane. Could you please:

- Include the patch in comment 0 as an attachment
- Detail what the impact/results/symptoms are of the incorrect calculation
- Provide confirmation (if possible) of the patch fixing the problem
Comment 2 Duane 2017-07-12 23:16:34 UTC
Created attachment 184314 [details]
Patch to remove warnings and align displayname with `xauth`
Comment 3 Duane 2017-07-12 23:23:29 UTC
Before the patch, running `startx` results in the following output:

======== 8< ========
# startx
xauth:  file /home/username/.serverauth.30545 does not exist
xauth: (argv):1:  bad display name "hostname:1" in "list" command
xauth: (stdin):1:  bad display name "hostname:1" in "add" command


X.Org X Server 1.18.4
Release Date: 2016-07-19
X Protocol Version 11, Revision 0
Build Operating System: FreeBSD 11.0-RELEASE-p10 amd64
Current Operating System: FreeBSD shuttle 11.1-RC1 FreeBSD 11.1-RC1 #0 r320486: Fri Jun 30 02:25:16 UTC 2017     root@releng2.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC amd64
Build Date: 06 July 2017  02:02:56AM

Current version of pixman: 0.34.0
        Before reporting problems, check http://wiki.x.org
        to make sure that you have the latest version.
Markers: (--) probed, (**) from config file, (==) default setting,
        (++) from command line, (!!) notice, (II) informational,
        (WW) warning, (EE) error, (NI) not implemented, (??) unknown.
(==) Log file: "/var/log/Xorg.1.log", Time: Wed Jul 12 17:41:54 2017
(==) Using config file: "/usr/local/etc/X11/xorg.conf"
xinit: connection to X server lost

waiting for X server to shut down (II) Server terminated successfully (0). Closing log file.

xauth: (argv):1:  bad display name "hostname:1" in "remove" command
======== 8< ========

After applying the patch the output is as follows:

======== 8< ========
# startx


X.Org X Server 1.18.4
Release Date: 2016-07-19
X Protocol Version 11, Revision 0
Build Operating System: FreeBSD 11.0-RELEASE-p10 amd64
Current Operating System: FreeBSD shuttle 11.1-RC1 FreeBSD 11.1-RC1 #0 r320486: Fri Jun 30 02:25:16 UTC 2017     root@releng2.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC amd64
Build Date: 06 July 2017  02:02:56AM

Current version of pixman: 0.34.0
        Before reporting problems, check http://wiki.x.org
        to make sure that you have the latest version.
Markers: (--) probed, (**) from config file, (==) default setting,
        (++) from command line, (!!) notice, (II) informational,
        (WW) warning, (EE) error, (NI) not implemented, (??) unknown.
(==) Log file: "/var/log/Xorg.1.log", Time: Wed Jul 12 17:43:30 2017
(==) Using config file: "/usr/local/etc/X11/xorg.conf"
xinit: connection to X server lost

waiting for X server to shut down (II) Server terminated successfully (0). Closing log file.
======== 8< ========

I don't know enough about `xauth` and all the rest to know the significance of the changes, and I'm not even sure when the different authority files come into play.  I do know the existing behaviour doesn't function as intended, but I cannot attest to whether the intended behaviour is correct.
Comment 4 Duane 2017-07-18 02:11:10 UTC
Created attachment 184458 [details]
Additionally fix `expr` bashism

This also fixes a bashism (I think) in the vtarg expression which was `expr match $i <regex>` and is now instead `expr $i : <regex>`.
Comment 5 Duane 2017-07-18 02:13:12 UTC
(In reply to Duane from comment #4)

Sorry, not a bashism, a gnuism.
Comment 6 Vladimir Kondratyev freebsd_committer 2017-07-18 07:41:58 UTC
(In reply to Duane from comment #4)
> `expr $i : <regex>`.

1. Parenthesises should be placed around $i to screen leading dashes in startx parameters: `expr \( $i \) : <regex>`
2. startx.cpp inside ${WRKSRC} directory is a right file to patch, not /usr/local/bin/startx
Comment 7 freelybsd 2017-07-22 19:45:44 UTC
I am not a programmer, but have been suspicious that this bug might exist; according to searches, many people are having trouble with it, especially since it is so unpredictable, in many Nixes. In Freebsd11.0, it has arisen for me every time that I load xfce. It generates an mtrr error as well. As it is pointed out here, the variable fails to reset.

I had not wanted to trouble you in this technical forum, but is there any way for a "user" with enough experience of Freebsd (5 years), vi, and Unix to enter into some file and repair it? I am frightened to do so without guidance.

So, if possible, would you be kind enough to provide specific instructions, starting in root (this is, x@y:/ #) and with the correct file path, to the file or files in question, and what to do when vi filename be invoked?

This would help many people.

Many thanks.
Comment 8 Matthew Rezny freebsd_committer 2017-07-30 11:23:10 UTC
Duane,

Thank you for pointing out these problems and suggesting fixes. It is not necessary to create the xserverauthfile with touch; xauth will create it on add / generate. The "does not exist" message is an expected informational message from the init function in xauth.

Vladimir,

You are correct on both points. Additionally, the regex needs to be translated to a BRE for BSD expr, i.e. '^vt[0-9]\+$' -> 'vt[0-9][0-9]*$'. There is an upstream commit which corrects the syntax in both regards (removes match and use of ERE syntax) but neglects the guard parenthesis.
Comment 9 Vladimir Kondratyev freebsd_committer 2017-07-30 14:44:16 UTC
(In reply to Matthew Rezny from comment #8)
> There is an upstream commit which corrects the syntax in both regards (removes match and use of ERE syntax) but neglects the guard parenthesis.

If I run startx with -nolisten tcp option and without parenthesis applied I get following error as expr treats -nolisten as own option:

$ /usr/local/bin/startx -- -nolisten tcp
expr: illegal option -- n
expr: usage: expr [-e] expression

This error is harmless but I dislike the message.
Other way to fix it is to place "x" before both string and regex:

if expr "x$i" : '^xvt[0-9]\+$'
Comment 10 Matthew Rezny freebsd_committer 2017-07-30 15:01:43 UTC
Created attachment 184847 [details]
fixup x11/xinit

Try with this patch.
Comment 11 commit-hook freebsd_committer 2017-08-27 19:28:20 UTC
A commit references this bug:

Author: rezny
Date: Sun Aug 27 19:27:38 UTC 2017
New revision: 448838
URL: https://svnweb.freebsd.org/changeset/ports/448838

Log:
  Set correct $displayname and remove expr GNUisms

  PR:		220668
  Reported by:	parakleta@darkreality.org, wulf

Changes:
  head/x11/xinit/Makefile
  head/x11/xinit/files/
  head/x11/xinit/files/patch-startx.cpp