Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#4845 closed defect (fixed)

wxURL can't download file with a path

Reported by: kervala Owned by:
Priority: high Milestone: 2.9.0
Component: network Version: stable-latest
Keywords: wxURI Cc: kervala
Blocked By: Blocking:
Patch: yes

Description

In wxURL, m_path member is full of garbage characters so GetInputStream fails.

The first method which returns a bad path is wxURI::ParsePath and inside this method, the code responsible for bad conversion seems to be :

Normalize(theBuffer, true); line 655 from src/common/uri.cpp

I don't understand the goal of this method so my temporary fix is to not normalize the path.

To reproduce this bug, I simply use :

wxURL data(_T("http://www.kervala.net/index.htm")); any URL with a path has the same problem

wxInputStream *input = data.GetInputStream(); <- crash

Thanks a lot for fixing this.

Cédric

Attachments (1)

uri.patch download (734 bytes) - added by kervala 6 years ago.
Patch for uri.cpp

Download all attachments as: .zip

Change History (16)

comment:1 Changed 7 years ago by kervala

Sorry, I forgot to tell I'm using SVN trunk version of wxWidgets :)

comment:2 Changed 6 years ago by kervala

This bug is still present in the last SVN trunk.

Nobody else is confrontated to this bug ?

comment:3 Changed 6 years ago by biol75

  • Status changed from new to infoneeded_new

Which platform/compiler? what did the debugger say about url.GetError() ?

[see wxProgressDialog * MyFrame::CheckServer at http://wxhatch.cvs.sourceforge.net/wxhatch/wxhatch/wxhatch.cpp?view=markup ]

seems ok to me
chris

comment:4 Changed 6 years ago by kervala

I'm sure there is no error because it crashs without any wxWidgets assert.

I'm using Visual C++ 2008 Express on Windows XP SP3.

wxWidgets 2.9.x is compiled in Unicode mode (by default).

I will check wxhatch tomorrow and post results, it's late here :)

comment:5 follow-up: Changed 6 years ago by kervala

  • Status changed from infoneeded_new to new

I checked the code from wxhatch and it didn't detect any error at url.GetError().

It crashed in wxInputStream * pInput = url1.GetInputStream() more especially near :

    wxString buf;
    buf.Printf(wxT("%s %s HTTP/1.0\r\n"), request, path.c_str());
    const wxWX2MBbuf pathbuf = buf.mb_str();
    Write(pathbuf, strlen(pathbuf)); // crash this line
    SendHeaders();
    Write("\r\n", 2);

path contains an invalid pointer to the string, so buf and pathbuf too.

comment:6 in reply to: ↑ 5 Changed 6 years ago by kervala

The code extract above is from http.cpp :)

comment:7 Changed 6 years ago by biol75

did you call wxFileSystem::AddHandler(new wxInternetFSHandler); before trying to read the server?
chris

comment:8 follow-up: Changed 6 years ago by kervala

I don't see why I should call wxFileSystem::AddHandler since I'm not accessing any file in a filesystem (or archive).

I tried your proposition and it did change anything.

Apparently, I need to provide a patch myself, even if I don't understand the goal of wxURI::Nomalize method.

So I'm working on a workaround for this "bug" and will post it there.

comment:9 in reply to: ↑ 8 Changed 6 years ago by kervala

Oops.

Instead of :

I tried your proposition and it did change anything.

You shoukd read :

I tried your proposition and it did NOT change anything.

Sorry.

Changed 6 years ago by kervala

Patch for uri.cpp

comment:10 Changed 6 years ago by kervala

I simply removed #if wxUSE_STL
wxUSE_UNICODE_UTF8 and #endif

I'm not using wxUSE_STL neither wxUSE_UNICODE_UTF8, but in theBuffer, m_buf needs to receive the data from m_str in this case too ! Otherwise, the string will only contain garbage characters.

If we don't make the copy with wxTmemcpy "theBuffer" couldn't be normalized).

I don't understand why this bug has not created any problem before...

comment:11 Changed 6 years ago by kervala

  • Keywords wxURI added
  • Milestone set to 2.9.0
  • Patch set
  • Priority changed from normal to high
  • Version set to 2.9-svn

comment:12 Changed 6 years ago by vadz

  • Status changed from new to confirmed

There is definitely a problem here, I'm just not sure if we always have to do the extra copy. And in fact I strongly suspect that there is a problem with wxStringBuffer classes now, looking at it...

comment:13 Changed 6 years ago by kervala

It could be possible :)

I also think the internal string has to be processed by wxStringBuffer classes (in its constructor or in (wxChar*) operator) and not by an external function.

Thanks a lot.

comment:14 Changed 6 years ago by vadz

  • Resolution set to fixed
  • Status changed from confirmed to closed

This should be fixed in the latest trunk (r53996), please reopen if it still doesn't work, thanks!

comment:15 Changed 6 years ago by kervala

It's working fine now, thanks a lot for fixing this.

Note: See TracTickets for help on using tickets.