From 653366fb23dee501807977b698e03eb38d5019a2 Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Sun, 5 Jul 2020 22:45:40 -0700 Subject: [PATCH] Fix mvim:// protocol double-encoding behavior handling properly Change mvim:// protocol behavior back to previous state to double-encode the file path, since we are encapsulating a file:// protocol (which has encoded path) within another URL as a query, which itself should be encoded. This means a file path "/tmp/file name.txt" should be properly encoded as mvim://open?url=file:///tmp/file%2520name.txt, as the space is encoded twice (first to %20, then to %2520). Previously we tried to fix the protocol handler to only do a single encoding (see #1021 and #1043) but it's really an incorrect usage of URL. The reason for that fix was that tools like iTerm2 was passing in single-encoded URLs. As such, also add a compatibility feature here where we will optimiscally try to re-encode characters that we detect to be erroneously encoded. For example, if we see mvim://open?url=file:///tmp/file%20name.txt, MacVim will intelligently realize that the space needs to be encoded again. The only character where that won't work is the "%" character because of the ambiguity involved, so a file path "/tmp/file%.txt" will only work with this: mvim://open?url=file:///tmp/file%2525.txt Close #1020 (also see the issue for discussions). --- runtime/doc/gui_mac.txt | 9 +++++ src/MacVim/MMAppController.m | 69 ++++++++++++++++++++---------------- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/runtime/doc/gui_mac.txt b/runtime/doc/gui_mac.txt index 6e0bd49b92..242403f3d0 100644 --- a/runtime/doc/gui_mac.txt +++ b/runtime/doc/gui_mac.txt @@ -604,6 +604,15 @@ For example, the link > mvim://open?url=file:///etc/profile&line=20 will open the file /etc/profile on line 20 when clicked in a web browser. +Note: A caveat in MacVim's implementation is that it expects special +characters to be encoded twice. For example, a space should be encoded into +"%2520" instead of "%20". A file "/tmp/file name?.txt" would need the +following link: > + mvim://open?url=file:///tmp/file%2520name%253F.txt + +MacVim will try to be smart and detect cases where a user has erroneously only +encoded once, but for best results use double-encoding as described above. + Note that url has to be a file:// url pointing to an existing local file. ============================================================================== diff --git a/src/MacVim/MMAppController.m b/src/MacVim/MMAppController.m index dddef7849b..64aec2133b 100644 --- a/src/MacVim/MMAppController.m +++ b/src/MacVim/MMAppController.m @@ -1767,29 +1767,39 @@ - (void)handleGetURLEvent:(NSAppleEventDescriptor *)event // parse value NSString *v = [arr objectAtIndex:1]; - // Ideally we don't decode anything here. The input parameters - // should be used as-in as there would be no reason for caller - // to encoder anything. For the line component it's a simple - // string, and the URL should already be a proper file:// URL - // with all the necessary characters (e.g. space) encoded and - // we can just pass it to NSURL as-in below. - // However, iTerm2 appears to encode the slashes as well - // resulting in URL that looks like - // file://%2Fsome%2Ffolder/file%20with%20space which is wrong - // as this doesn't form a valid URL. To accommodate that, we - // decode the URL, and later on manually parse it instead of - // relying on NSURL. - // See: https://github.com/macvim-dev/macvim/issues/1020. - - // BOOL decode = ![f isEqualToString:@"url"]; - const BOOL decode = YES; - - if (decode) - { + // We need to decode the parameters here because most URL + // parsers treat the query component as needing to be decoded + // instead of treating it as is. It does mean that a link to + // open file "/tmp/file name.txt" will be + // mvim://open?url=file:///tmp/file%2520name.txt to encode a + // URL of file:///tmp/file%20name.txt. This double-encoding is + // intentional to follow the URL spec. #if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_11 - v = [v stringByRemovingPercentEncoding]; + v = [v stringByRemovingPercentEncoding]; #else - v = [v stringByReplacingPercentEscapesUsingEncoding:NSUTF8StringEncoding]; + v = [v stringByReplacingPercentEscapesUsingEncoding:NSUTF8StringEncoding]; +#endif + + if ([f isEqualToString:@"url"]) { + // Since the URL scheme uses a double-encoding due to a + // file:// URL encoded in another mvim: one, existing tools + // like iTerm2 could sometimes erroneously only do a single + // encode. To maximize compatiblity, we re-encode invalid + // characters if we detect them as they would not work + // later on when we pass this string to URLWithString. + // + // E.g. mvim://open?uri=file:///foo%20bar => "file:///foo bar" + // which is not a valid URL, so we re-encode it to + // file:///foo%20bar here. The only important case is to + // not touch the "%" character as it introduces ambiguity + // and the re-encoding is a nice compatibility feature, but + // the canonical form should be double-encoded, i.e. + // mvim://open?uri=file:///foo%2520bar +#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_11 + NSMutableCharacterSet *charSet = [NSMutableCharacterSet characterSetWithCharactersInString:@"%"]; + [charSet formUnionWithCharacterSet:NSCharacterSet.URLHostAllowedCharacterSet]; + [charSet formUnionWithCharacterSet:NSCharacterSet.URLPathAllowedCharacterSet]; + v = [v stringByAddingPercentEncodingWithAllowedCharacters:charSet]; #endif } @@ -1800,12 +1810,9 @@ - (void)handleGetURLEvent:(NSAppleEventDescriptor *)event // Actually open the file. NSString *file = [dict objectForKey:@"url"]; if (file != nil) { - // Instead of passing "file" to NSURL directly, we just manually - // parse the URL because the URL is already decoded and NSURL will - // get confused by special chars like spaces. See above - // explanation. - if ([file hasPrefix:@"file:///"]) { - NSString *filePath = [file substringFromIndex:7]; + NSURL *fileUrl = [NSURL URLWithString:file]; + if ([fileUrl isFileURL]) { + NSString *filePath = [fileUrl path]; // Only opens files that already exist. if ([[NSFileManager defaultManager] fileExistsAtPath:filePath]) { NSArray *filenames = [NSArray arrayWithObject:filePath]; @@ -1847,11 +1854,11 @@ - (void)handleGetURLEvent:(NSAppleEventDescriptor *)event [alert addButtonWithTitle:NSLocalizedString(@"OK", @"Dialog button")]; - [alert setMessageText:NSLocalizedString(@"Unknown File Protocol", - @"Unknown File Protocol dialog, title")]; + [alert setMessageText:NSLocalizedString(@"Invalid File URL", + @"Unknown Fie URL dialog, title")]; [alert setInformativeText:[NSString stringWithFormat:NSLocalizedString( - @"Unknown protocol in \"%@\"", - @"Unknown File Protocol dialog, text"), + @"Unknown file URL in \"%@\"", + @"Unknown file URL dialog, text"), file]]; [alert setAlertStyle:NSAlertStyleWarning];