[tor-bugs] #9957 [Tor]: Tor should consider stderr output of transport proxies

[Tor Bug Tracker & Wiki]

#9957: Tor should consider stderr output of transport proxies
------------------------+--------------------------------
     Reporter:  wfn     |      Owner:
         Type:  defect  |     Status:  needs_revision
     Priority:  minor   |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  tor-pt
Actual Points:          |  Parent ID:
       Points:          |
------------------------+--------------------------------

Comment (by wfn):

 Replying to [comment:20 asn]:
 > Replying to [comment:19 wfn]:
 > > Replying to [comment:17 asn]:
 > > > {{{
 > > >     log_notice(LD_GENERAL, "Managed proxy '%s' is giving some output
 over "
 > > >                "the standard error stream (stderr). This usually
 means that "
 > > >                "it has encountered some error that I won't be able
 to "
 > > >                "understand. You should enable INFO-level tor log to
 see what "
 > > >                "it is saying - when INFO-level log is enabled, look
 for "
 > > >                "lines with 'report_proxy_stderr' (without quotes).",
 > > >                mp->argv[0]);
 > > > }}}
 > > >
 > > > Hm, does this mean that the info-level logs will have the string
 `report_proxy_stderr` in them? Is this the case in the current code?
 > >
 > > Yes - e.g. look above at the log excerpt I pasted in.
 > > So e.g.
 > > {{{
 > > Apr 19 06:46:51.000 [info] report_proxy_stderr(): Managed proxy
 '/usr/local/bin/obfsproxy' says (over stderr): "Traceback (most recent
 call last):"
 > > }}}
 > >
 > > Surely this is not platform-specific?
 > >
 >
 > Ah, because of `should_log_function_name()`, I guess.

 Aha, I assume it's OK to rely on should_log_function_name() not changing
 in the foreseeable future (i.e. `LOG_DEBUG` and `LOG_INFO`-bound messages
 should always include function names. ref. `log.c`)

 > Changelog message looks good to me.
 >
 > BTW, you don't need to declare the static function in `transports.h`.
 Just declare it on the top of `transports.c` along with
 `handle_finished_proxy()` etc.
 >
 > Finally:
 > {{{
 >     /* Should we expect any output from stderr to contribute to the
 proxy
 >      * config process? (We assume not.) */
 >     log_info(LD_GENERAL, "Managed proxy '%s' says (over stderr): %s",
 >              mp->argv[0], escaped(line));
 >     /* (If we want to just output this stream in one go, we may use
 >      * get_string_from_pipe() - but this works, too.) */
 > }}}
 > Nitpick, but I'm not sure if the last comment is any useful.
 >
 > Marking this as needs_revision for the function declaration and changes
 file.

 Ok. Rebased as per your comments (the function declarations at the top of
 `transports.c` are not really in an line-they-are-defined-in-increasing-
 order, so placed the declaration where it seemed appropriate (nitpicking
 level > 9000, but hey)), included the changes file in commit.

 => https://github.com/wfn/tor/compare/bug_9957_2

 let me know if I missed something.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9957#comment:21>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online