Opened 4 years ago

Closed 4 years ago

#11508 closed defect (fixed)

FIX for wxLaunchDefaultApplication not working on OSX if path contains spaces

Reported by: SnowLeopard Owned by: csomor
Priority: normal Milestone: 2.9.1
Component: wxOSX-Cocoa Version: stable-latest
Keywords: filename quote simple Cc:
Blocked By: Blocking:
Patch: yes

Description

On OSX, if you call wxLaunchDefaultApplication with a document path which contains spaces, then it will fail. This is because the document path is not wrapped in quotes when sent as a command line to "open". This patches corrects that.

Attachments (1)

update.diff download (495 bytes) - added by SnowLeopard 4 years ago.

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by SnowLeopard

comment:1 Changed 4 years ago by vadz

  • Keywords filename quote simple added
  • Milestone set to 2.9.1
  • Status changed from new to confirmed

I wonder what happens if the path contains quotes, shouldn't they be escaped too (by using backslashes I assume)?

Actually I thought we already had a function for quoting file names somewhere but I can't find it, the closest I see is wxFileType::ExpandCommand() but the quoting code there is (a) trivial and (b) commented out. I wonder if we shouldn't add a function to do this as I'm still almost certain that we needed it somewhere else, too...

comment:2 Changed 4 years ago by SnowLeopard

In terms of quotes being in the file path, I don't think quotes can be used in a file or folder name, surely that's an illegal character on any platform.

In terms of a filepath already being wrapped in quotes, that might be a problem, not sure. You could have a function that checks the first and last indices in the string and see if they are quotes, and if not then wrap it in quotes.

comment:3 Changed 4 years ago by vadz

Under Unix systems traditionally the only invalid characters in the file names are / (ASCII 0x2f) and NUL (ASCII 0). I'm pretty sure OS X should allow quotes in the file names too. It's probably not very common to use them, of course, but if we want to have a properly quoting function we need to do it right.

Anyhow, thinking further about it, the proper fix is not to do any quoting at all in wxLaunchDefaultApplication() itself. Instead it should use the wxExecute() overload taking argv array instead of a string, i.e.:

wchar_t argv[2];
argv[0] = L"/usr/bin/open";
argv[1] = document.wc_str();
wxExecute(argv);

This should work, shouldn't it? Could you please test if it does?

comment:4 Changed 4 years ago by SnowLeopard

wxExecute taking a "wchar_t" instead of "const wchar_t" really makes things complicated. That code won't work, I tried copying those strings into an array of newly mallocs strings and passed those to wxExecute, but I keep crashing. Here is my stab at it:

wchar_t* argv[2];
    argv[0] = (wchar_t*)malloc(14);
    wmemset(argv[0], 0, 14);
    wcsncpy(argv[0], L"/usr/bin/open", 13);

    argv[1] = (wchar_t*)malloc(document.length()+1);
    wmemset(argv[1], 0, document.length()+1);
    wcsncpy(argv[1], (const wchar_t*)document, document.length());

    bool retval = wxExecute(argv);
    free(argv[0]);
    free(argv[1]);
    return retval;

I don't know, I think my fix of just wrapping the doc path in quotes is simpler (and doesn't crash).

comment:5 Changed 4 years ago by vadz

wxExecute() probably should have const overloads but the code above should work too, I don't know why does it crash.

I'd like to understand this and fix this 100% correctly but if I don't manage to do it before 2.9.1 feel free to apply the original patch which is definitely better than nothing even if it's not ideal.

comment:6 Changed 4 years ago by csomor

  • Owner set to csomor
  • Status changed from confirmed to accepted

as Vadim said, quotes are perfectly valid on OSX, so I'll switch to an implementation using LaunchServices, this will properly work with any valid filename

comment:7 Changed 4 years ago by SC

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

(In [63181]) switching to LaunchServices implementation, fixes #11508

Note: See TracTickets for help on using tickets.