#14336: wxHTTP::Connect to localhost always returns true.

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------
When trying to connect to localhost, wxHTTP::Connect always returns true
no matter if a server is running or not:

{{{
wxHTTP::SetHeader(wxT("Content-type"), wxT("text/html; charset=utf-8"));
wxHTTP::SetTimeout(30);

if (!wxHTTP::Connect(wxT("localhost")))
return false;
}}}

In this example, false is never returned although IsConnected() will fail.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:1>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by vadz):

Could you please try to debug what happens inside `Connect()`?

Also, which platform is this under?


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:1>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:2>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by Silverstorm82):

I am on it.

It happened using Win XP and Win 7.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:2>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:3>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by Silverstorm82):

Looks like

{{{
return GetImpl().SetHostName(name);
}}}

in sckaddr.cpp wxIPaddress::Hostname() returns always true for localhost
whereas for other servers false ist returned.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:3>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:4>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by Silverstorm82):

Update:
In sckaddr.cpp wxGethostbyname_r() the line

{{{
he = gethostbyname(hostname);
}}}

returns the Windows computer name as the hostname whereas for other
invalid hostnames it returns an error.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:4>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:5>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by Kry):

This is because wxHTTP::Connect does not actually connect, it just
resolves the hostname and sets the "Host:" header for future usage.
Localhost obviously resolves always.

It literally does "return true;" if the hostname resolves, independently
of reachability.

The actual connection is done when wxHTTP::GetInputStream is called.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:5>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:6>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by Silverstorm82):

That explains why IsConnected() returns false even Connect() returns true.

However, isn't it somehow "strange" that Connect() can be executed
successfully but IsConnected() returns false afterwards?


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:6>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:7>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by Kry):

It sure is.

wxHTTP, which is a subclass of wxProtocol, which is a subclass of
wxSocketClient, ideally would follow the implementation of
wxSocketClient::Connect (
http://docs.wxwidgets.org/trunk/classwx_socket_client.html#a581cdb757cce6020c8caac1ddd74a599
). Unfortunately, it doesn't - and it wouldn't make much sense to make it
so, unless all the headers/etc were set first. Basically Connect() is a
misnomer here.

It would be possible to initiate the connection on Connect(), then send
the headers and create the input stream in GetInputStream, but I don't
know what the best solution is here to pound some sense into the
interface.

If there's some discussion about any changes in the interface or
functionality by other wx developers, I officially offer to make the
changes myself.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:7>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:8>

#14336: wxHTTP::Connect to localhost always returns true.
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Changes (by Kry):

* cc: kry@… (added)



--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:8>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:9>

#14336: wxHTTP::Connect() doesn't really open connection to the server
------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: confirmed
Priority: low | Milestone:

Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Changes (by vadz):

* priority: normal => low
* status: new => confirmed


Comment:

It would definitely make sense for `Connect()` to open the connection. I
think we could do this even before `SetHeader()` is called -- it's fine to
open a connection but only write something to it a bit later, why not?


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:9>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:10>

#14336: wxHTTP::Connect() doesn't really open connection to the server

------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: confirmed
Priority: low | Milestone:

Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by Kry):

Then I suggest changing

{{{
virtual bool Connect (const wxString &host)
virtual bool Connect (const wxString &host, unsigned short port)
}}}

to

{{{
virtual bool Connect (const wxString &host, unsigned short port = 0,
bool wait = true)
}}}

As I don't see the point for the two overloads, and we'll need to add the
wait parameter (currently unused in virtual bool Connect(const
wxSockAddress& addr, bool wait); )

Port 0 means for the Connect() function to choose the http port, we could
also make it default to 80 but 0 triggers else branch the following code:

{{{
if ( port )
addr->Service(port);
else if (!addr->Service(wxT("http")))
addr->Service(80);
}}}

which somehow looks like there's a point to it, even if I can't see it
(it's not like Service(wxT("http")) is ever going to be other than 80).

Once this change is made, I can modify

{{{
bool wxHTTP::Connect(const wxSockAddress& addr, bool WXUNUSED(wait))
}}}

to actually call the wxSocketClient part for either busy connection (with
wait == true) or the asynchronous one. And of course modify GetInputStream
properly.

If this seems like a good solution, I'll write and document it.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:10>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:11>

#14336: wxHTTP::Connect() doesn't really open connection to the server

------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:
Type: defect | Status: confirmed
Priority: low | Milestone:

Component: network | Version: 2.9-svn
Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by vadz):

I don't see any problems with this but OTOH this code has a lot of
unpleasant surprises so I can't be sure of not missing something...


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:11>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:12>

#14336: wxHTTP::Connect() doesn't really open connection to the server

------------------------------+---------------------------------------------
Reporter: Silverstorm82 | Owner:

Type: defect | Status: confirmed
Priority: low | Milestone:

Component: network | Version: stable-latest

Keywords: wxHTPP localhost | Blockedby:
Patch: 0 | Blocking:
------------------------------+---------------------------------------------

Comment(by robertthomas):

The most up to date listing of coupons for [http://www.couponsdeals.in/
shopping]. Our editors check coupon codes to ensure validity every day.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14336#comment:12>