[patch] fix broken --stdin-stdout option that writes to stdin

[Israel G. Lugo]

Binary package hint: httptunnel

The --stdin-stdout option is broken, both in the client (htc) and the server (hts). When used, the program writes its output to stdin instead of stdout. This breaks anything calling htc or hts with --stdin-stdout, unless stdin happens to be something which can be written to (e.g. a socket or a pseudo-terminal).

In particular, it will break programs that communicate with htc or hts with via pipes, such as the OpenSSH client with the ProxyCommand set to htc (which is one of the main purposes for which someone would want to use httptunnel in the first place).

What happens is an infinite connect/disconnect loop where htc connects, gets the banner from the server (sshd for example), fails to pass it to the client (ssh in this case) because it tries to write to stdin, then reconnects to try again, over and over.

To reproduce, take the following steps:

1. Run an httptunnel server somewhere, for example:

     $ hts -F www.example.com:80 -w

2. Run the client with --stdin-stdout, so that its stdin is not a tty
   (in this case, it's a pipe from cat). Assuming the server is on
   the same machine:

     $ cat | htc --stdin-stdout localhost

3. While the client is still running, check the active connections.
   You will notice hundreds of connections being created and closed,
   endlessly.

     $ netstat -at

The expected behavior would be for htc not to be stuck making infinite connections to hts, and for the tunnel to actually work. As it stands, if you type e.g. "GET / HTTP/1.0" (without the quotes) and two newlines on the client at step 2, nothing will happen (you should see the HTML response from www.example.com).

The problem is present in the latest version of the httptunnel package, and it is present in the latest upstream version as well (3.3 as of this writing, since 2001). I have reported it to the upstream maintainers via email and sent them the same patch I am submitting below.

[Israel G. Lugo (ilugo)]

Patch to fix the problem (caused by a broken check in the source).

ChangeLog entry:

2007-06-20 Israel G. Lugo <email address hidden>

    * common.c (handle_tunnel_input): really write to stdout if fd == 0.

[Daniel Hahler (blueyed)]

I've created a debdiff with your patch.

But I'm confused about the changes to config.sub and config.guess in the debdiff. So this may not be "ready".

Additionally, it may be the best option to report it upstream/at Debian, because there are no Ubuntu related changes in the package yet.

[Israel G. Lugo (ilugo)]

Understood. I have reported the bug to Debian, it can be seen on http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=429885 (as is incidentally now indicated in the bug status above)

[Daniel Hahler (blueyed)]

httptunnel (3.3-3ubuntu1) gutsy; urgency=low

  * common.c (handle_tunnel_input): really write to stdout if fd == 0.
    LP: #121458
  * Changed Maintainer field according to
    https://wiki.ubuntu.com/DebianMaintainerField

 -- dAniel hAhler <email address hidden> Thu, 21 Jun 2007 01:58:21 +0200

[Israel G. Lugo (ilugo)]

Hmm, no attribution to the bug reporter/patch creator?

[Israel G. Lugo (ilugo)]

No answer, I see. Is this really how things work at Ubuntu? It would have been polite, not to mention proper, to at least name the person who found the problem, corrected it and bothered to send the solution to you. So the patch and even the changelog entry are good enough to use, but not the name of the person who came up with it in the first place? Is this how Ubuntu encourages people to contribute? Bad form, very bad form.

[Daniel Hahler (blueyed)]

Sorry, Israel. Of course, this is not how things work at Ubuntu.

I've just created the debdiff which is required to get your patch into the distribution and have forgotten to include your name therein (this is one of my first debdiffs).
As you may have noticed, I was not sure about the procedure at all - and therefor only have provided the debdiff. I have not uploaded it myself.
If I understand this correctly, Debian should also apply this patch. When they would mention you in the changelog, Ubuntu would merge/use the changelog from Debian, when syncing the package again.

Please note, that I only wanted to help out, by quickly providing a debdiff - somehow only as an exercise and my intention was not to disrespect anyone.

I will look into getting this into upstream (e.g. to Debian) tomorrow and then take care about proper credit.

[Luke Yelavich (themuso)]

This was also an oversight on my part, as the one who reviewed and
uploaded the change. I should have asked for the debdiff to be
resubmitted with credit given where credit was due, but obviously my
mind wasn't on the job as well as I would have liked at the time.

I would like to apologise for my mistake, and will certainly make sure
that this mistake doesn't get made again, at least on my part. Not
giving credit where its due is certainly not the Ubuntu norm, so any
criticism that is leveled, is leveled at me, which I take
constructively.

Any help that the Ubuntu project gets is welcomly received, and should
be praised where possible, so thanks to both Israel G. Lugo, and dAniel
hAhler for the patches submitted.

[Israel G. Lugo (ilugo)]

Thank you for your responses, dAniel and Luke. I appreciate the clarification.

Of course I did not believe this to be the norm in a great distribution like Ubuntu, and I am happy to contribute to such a worthy project. I was frustrated with what happened, though, especially since I did not hear anything for several days after asking (even though the update itself was out within a day of initial report).

Given that this was only a simple oversight, I assume that there is some way to make things right... dAniel's idea would seem like a good course of action.

Regards,
Israel