Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure file paths are properly escaped during pasteboard paste operations #5036

Conversation

AlexJuca
Copy link
Contributor

This PR ensures file paths derived from pasteboard operations are properly escaped:

Before fix:

Attempting to cd into these paths rendered the following error: cd: too many arguments because the directory name includes spaces or special characters, which need to be handled correctly when running the cd command.

Screen.Recording.2025-01-13.at.23.55.23.video-converter.com.mp4

Afer fix

File paths are now properly escaped and we can correctly cd into these paths.

Screen.Recording.2025-01-13.at.23.58.00.video-converter.com.mp4

This change ensures Ghostty has the same behaviour as Iterm2.

@AlexJuca AlexJuca force-pushed the fix-ensure-file-path-is-escaped-pasteboard-url-handling branch from ac45070 to 45b472c Compare January 13, 2025 23:17
@AlexJuca AlexJuca force-pushed the fix-ensure-file-path-is-escaped-pasteboard-url-handling branch from 45b472c to 39bb949 Compare January 13, 2025 23:18
@Mil0dV
Copy link

Mil0dV commented Jan 13, 2025

Ok so I don't know this code base enough to figure out if this also applies to linux (I searched and found bits of safepaste code, but I tested it, and it fails in exactly the same way.

So I'd say: why not also fix this for linux?

@AlexJuca
Copy link
Contributor Author

Ok so I don't know this code base enough to figure out if this also applies to linux (I searched and found bits of safepaste code, but I tested it, and it seems to fail exactly like you described (vids won't load for me).

So I'd say: why not also fix this for linux?

@Mil0dV I don't know if this issue exists for Linux.

Can you reproduce the issue on Linux? That would be nice.

Unfortunately, I can't because I don't use Linux, and the fix will likely involve very platform-specific changes.

@mitchellh mitchellh enabled auto-merge January 14, 2025 03:47
@mitchellh mitchellh merged commit d1fd22a into ghostty-org:main Jan 14, 2025
30 checks passed
@github-actions github-actions bot added this to the 1.1.0 milestone Jan 14, 2025
@AlexJuca AlexJuca deleted the fix-ensure-file-path-is-escaped-pasteboard-url-handling branch January 14, 2025 07:15
@Mil0dV
Copy link

Mil0dV commented Jan 14, 2025

@AlexJuca ah my edit and your reply crossed eachother. Yes, I reproduced this on linux. Shouldn't the same Ghostty.Shell.escape() also work there, if the right file is found to insert it?

@AlexJuca
Copy link
Contributor Author

@AlexJuca ah my edit and your reply crossed eachother. Yes, I reproduced this on linux. Shouldn't the same Ghostty.Shell.escape() also work there, if the right file is found to insert it?

@Mil0dV The Ghostty.Shell.escape() function is a Swift only API, so it won't work on Linux.

@jcollie
Copy link
Collaborator

jcollie commented Jan 14, 2025

This was recently added to the Zig code:

pub fn ShellEscapeWriter(comptime T: type) type {
return struct {
child_writer: T,
fn write(self: *ShellEscapeWriter(T), data: []const u8) error{Error}!usize {
var count: usize = 0;
for (data) |byte| {
const buf = switch (byte) {
'\\',
'"',
'\'',
'$',
'`',
'*',
'?',
' ',
'|',
=> &[_]u8{ '\\', byte },
else => &[_]u8{byte},
};
self.child_writer.writeAll(buf) catch return error.Error;
count += 1;
}
return count;
}
const Writer = std.io.Writer(*ShellEscapeWriter(T), error{Error}, write);
pub fn writer(self: *ShellEscapeWriter(T)) Writer {
return .{ .context = self };
}
};
}

It's not exposed to macOS at the moment though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants