Patrick Schleizer transcribed 2.2K bytes:
> isis:
> > This should be fixed (for Linux) in an upcoming Tor Browser 4.0 release. I've
> > added these things to the `start-tor-browser` script. There are:
> >
> > - Instructions for use, including additional Firefox preferences that
> > you'll need to set (to tell Tor Button where your ControlPort and
> > SocksPort are, etc.)
> >
> > - An "easy" spot in the `start-tor-browser` script to put in your
> > ControlPort password so that it is given to the Firefox process (but not
> > exported, because then other programs might be able to steal it)
> >
> > - A line will print to your terminal telling you that you're using a
> > system-installed Tor.
> >
> > See #10178, [0] and this commit [1] containing the changes. If you're running
> > Linux and would like to test these, nightly builds are available. [2]
> >
> > Contributions to improve this, i.e. automatically setting the preferences for
> > you, are welcome.
> >
> > [0]: https://bugs.torproject.org/10178
> > [1]: https://gitweb.torproject.org/builders/tor-browser-bundle.git/commitdiff/a566e85f785c12157283920a179cdc64fdd28b32
> > [2]: https://people.torproject.org/~linus/builds/
>
> Hi isis,
>
> thank you for working on this!
>
> Comments on the script / patch:
Hey Patrick!
Thanks for the thanks, and for the code review!
> - You switched from #!/bin/sh to #!/bin/bash. While I personally
> preferred to forget about bothering with sh and switching to bash, I
> don't there are any systems that only have sh, no bash? (`checkbashisms`
> may be helpful.)
tl;dr: I've never used that devscript before. Neat, thanks! You're actually
correct that `/bin/bash` doesn't need to be there, because my changes which
required it [0] were reverted for TB-4.0-alpha-1 [1] due to the concerns
(mostly the fault of HTTPSEverywhere logging URIs) raised by Robert Ransom on
ticket #12468 [2] (although they are still in 3.6.x). To read more about why I
wrote this crazy code, read on. :)
I think I probably know most of the bashisms, like `[` being a command, you
can still `exec 3<>/dev/tcp/fyb.patternsinthevoid.net/80` to open a TCP
connection even if there isn't a `/dev/tcp`, [3] doing `echo '-e'` doesn't
echo anything because echo is a broken piece of crap... I can keep going. :)
Bash is messed up. Usually, I know how to convert bash syntax to shell, but
there are a couple builtins in particular which don't really have a shell
equivalent.
Normally, I would not want to switch to bash due to portability issues, as you
point out (and unstable syntax and behaviour between bash versions). But the
the particular bashism that I needed in this case, there isn't any equivalent
(to my knowledge) in shell: the `disown` builtin. It's used in the very last
stanzas that I added to the start-tor-browser script:
for second in `seq 1 15` ; do
sleep 1
if `kill -0 $pid 2>&1 >/dev/null ` ; then
wait "$pid"
exitcode="$?"
printf "Tor Browser exited suddenly! Exit code: %s\n" "$exitcode"
exit "$exitcode"
else
continue
fi
done
if test -z "${exitcode}" ; then
if test -z "$(kill -0 $pid 2>&1 >/dev/null)" ; then
printf "Running Tor Browser process (PID %s) in background...\n" "$pid"
disown "$pid"
exitcode="0"
else
exitcode="66" # Something odd happened
fi
fi
exit "$exitcode"
For additional reasoning behind this crazy code, see my commit message. [0]
> - Can you run the script through the code analysis tool `shellcheck`
> please? [1] It reports some issues that are worth fixing.
Hmm, like most static analysis tools, it seems like it's not really doing a
good job at taking overall context into account:
1. There are plenty of cases where I *do* want ${@} to glob and split words,
that's *why* there's no quotes around it.
2. It doesn't seem to know about `test -t` syntax to test if a file
descriptor is opened on a terminal.
3. And it seems to be confused on the difference between $(...) and `...`
syntax. $(...) is evaluated at access time (if unquoted), `...` is
evaluated immediately. There's a good Bash Hackers article on this [4]
(the writer actually concludes on the $(...) side of things too, like the
shellcheck tool), but I really think it's a style preference. I could
easily argue that because:
$( (ls) )
and
$((ls))
are entirely different commands, that therefore $(...) syntax is
confusing and tricky and should be considered deprecated. Also, while
`...` doesn't handle nested backticks nicely, its *real* POSIX equivalent
"$(...)" (with quotes) doesn't handle nested double-quotes nicely. :)
Though `shellcheck` is actually pointing out some valid mistakes. I'll have to
ask Mike Perry and the rest of the Tor Browser Team what they want done with
that. (I think there were some major changes planned, and the
start-tor-browser script is going to deprecated/replaced?)
Either way, you seem really knowledgable about shell scripting, if you wanted
to contribute to Tor Browser, using your skills on all the bash/shell in the
the tor-browser-bundle repo [5] would be a great way to start.
> - Wouldn't it be better if the script started Firefox using `exec`? (As
> per best scripting practices [4].)
In the case of the old (pre TB-3.6.2, before my crazy `wait` code above),
possibly. But doing so would obviously mean that the `firefox` process and its
environment would replace the shell. This means that when the `firefox`
process dies, the shell would disappear (along with any advice, errors, logs
which had been logged to it, since the old version doesn't log to file).
In the versions which do have my crazy code, that crazy code wouldn't get run,
because there wouldn't be a shell to run it any longer. I felt that the crazy
code was necessary, as it gives error messages for the potential cases where
Firefox couldn't start because it was already running, couldn't start period,
exited unexpectedly, or couldn't be disowned for some reason. In all cases, my
crazy code *should* handle dealing with hung, non-existent, errored,
etc. Firefox processes, and then it *should* always exit the shell, which (to
my understanding) is pretty much the reason one is supposed to `exec` a
wrapped process, so that an extra (sub)shell process isn't left lying around.
I could be entirely wrong, though, and perhaps there is some fancy `exec`
syntax which takes care of the processes errors. (If there is, I'd be stoked
to learn about it!)
> If you wish I could do the changes in a github branch if you would be
> willing to review.
That sounds great! Just be sure to explain weird shellisms and bashisms with
code comments or commit messages.
I also see you have an (somewhat) old tbb-scripts repo. [6] I've got a bunch
of random hacky tor-related scripts too, for setting up transproxies and
catching GFW garbage probes, [7] verifying gitian builder scripts, [8] doing
all your DNS on a remote Tor relay (through tor, with DNSSEC validation and
caching), [9] etc. I know Ximin's (infinity0) got some nice hacky scripts too,
and meejah, perhaps we should all join forces and create a tor-hacks repo?
> All the best,
> Patrick
>
> [1] shellcheck [2] is an online service and Libre Software
> (downloadable) that detects problems with sh/bash scripts. Also
> available in Debian jessie [3] and sid.
> [2] http://www.shellcheck.net/
> [3] https://packages.debian.org/jessie/shellcheck
> [4] http://mywiki.wooledge.org/WrapperScript
>
[0]: https://gitweb.torproject.org/builders/tor-browser-bundle.git/commitdiff/c2bd8a5f070e771623aebfb66e95e9e954e5522d
[1]: https://gitweb.torproject.org/builders/tor-browser-bundle.git/commit/27f1353baa0e3ce4dafcebe0eae6b48aa2f8704c
[2]: https://trac.torproject.org/projects/tor/ticket/12468
[3]: http://www.linuxjournal.com/content/more-using-bashs-built-devtcp-file-tcpip
[4]: http://wiki.bash-hackers.org/syntax/expansion/cmdsubst
[5]: https://gitweb.torproject.org/builders/tor-browser-bundle.git
[6]: https://github.com/adrelanos/tbb-scripts
[7]: https://github.com/isislovecruft/scripts/blob/master/transproxy.firewall.sh
[8]: https://github.com/isislovecruft/scripts/blob/master/verify-gitian-builder-signatures
[9]: https://github.com/isislovecruft/scripts/blob/master/dnswithtor
--
ââ isis agora lovecruft
_________________________________________________________
GPG: 4096R/A3ADB67A2CDB8B35
Current Keys: https://blog.patternsinthevoid.net/isis.txt
Attachment:
signature.asc
Description: Digital signature
-- tor-talk mailing list - tor-talk@xxxxxxxxxxxxxxxxxxxx To unsubscribe or change other settings go to https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-talk