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

Escape URLs on Windows #11

Merged
merged 4 commits into from
Dec 17, 2018
Merged

Conversation

qryxip
Copy link
Contributor

@qryxip qryxip commented Dec 14, 2018

In the current implementation on Windows, this crate cannot open a URL which has special characters for cmd.exe. (e.g. http://example.com?q1=0&q2=0)
This PR fixes the problem by escaping URLs.

@amodm
Copy link
Owner

amodm commented Dec 15, 2018

@wariuni thanks for the contribution. Can you elaborate if powershell is a default installation in various Windows versions? Which versions is it not present in?

@qryxip
Copy link
Contributor Author

qryxip commented Dec 16, 2018

On Windows 7 at least PowerShell v2.0 seems to be pre-installed, which contains Start-Process command.
So in this implementation it probably works on all versions of supported desktop Windows.

https://blogs.technet.microsoft.com/josebda/2009/11/25/download-for-powershell-v2-for-windows-7-no-need-its-already-there/
https://docs.microsoft.com/en-us/powershell/scripting/install/windows-powershell-system-requirements?view=powershell-6

But on second thought, there is no specail reason to replace cmd with powershell.
To escape a string on cmd, we only have to prefix a ^ to every characters.
I thought it has hard.

Incidentally, I came up with another way to open a URL on Windows.
By using winapi crate, we can open a URL via ShellExecuteW in the same way as CPython does.

https://github.com/python/cpython/blob/8c281ed403fd915284d5bba2405d7c47f8195066/Lib/webbrowser.py#L581
https://github.com/python/cpython/blob/afb3e71a1710c444fbe789b51df43ee16ee9ede7/Modules/posixmodule.c#L11241-L11242
https://docs.microsoft.com/en-us/windows/desktop/api/shellapi/nf-shellapi-shellexecutew

What do you think is the best way?

  • cmd
    • Pros
      • cmd.exe certainly exists in every desktop Windows
      • (Additional) Come to think of it, it is easy to escape characters
    • Cons
      • Possibly fails to open a URL which contains non-ASCII characters. (probably doesn't matter)
        We can encode non-ASCII chars by using percent-encoding crate
  • PowerShell
    • Cons
      • Possibly does not work on old versions of Windows.
      • Possibly fails to open a URL which contains non-ASCII characters
  • ShellExecuteW
    • Pros
      • Certainly works
    • Cons
      • Requires at least winapi
      • Two (or one?) unsafe functions
//! ```cargo
//! [package]
//! name = "startfile"
//! edition = "2018"
//!
//! [target.'cfg(windows)'.dependencies]
//! env_logger = "0.6.0"
//! log = "0.4.6"
//! percent-encoding = "1.0.1" # part of `url` crate
//! structopt = "0.2.14"
//! strum = "0.12.0"
//! strum_macros = "0.12.0"
//! widestring = "0.4.0"
//! winapi = { version = "0.3.6", features = ["combaseapi", "objbase", "shellapi"] }
//! ```
#![allow(unreachable_code)]

#[cfg(not(windows))]
compile_error!("");

use log::info;
use structopt::StructOpt;
use strum_macros::EnumString;
use widestring::U16CString;

use std::borrow::Cow;
use std::process::{Command, Stdio};
use std::{io, ptr};

#[derive(StructOpt)]
struct Opt {
    file: String,
    #[structopt(short = "m", long = "method", default_value = "winapi")]
    method: Method,
}

#[derive(EnumString)]
#[strum(serialize_all = "snake_case")]
enum Method {
    Winapi,
    Cmd,
    Powershell,
}

fn main() -> io::Result<()> {
    env_logger::init();
    let Opt { file, method } = Opt::from_args();
    match method {
        Method::Winapi => startfile_with_winapi(&file),
        Method::Cmd => startfile_with_cmd(&encode_if_url(&file)),
        Method::Powershell => startfile_with_powershell(&encode_if_url(&file)),
    }
}

fn startfile_with_winapi(file: &str) -> io::Result<()> {
    // https://docs.microsoft.com/en-us/windows/desktop/api/shellapi/nf-shellapi-shellexecutew

    use winapi::um::combaseapi::CoInitializeEx;
    use winapi::um::objbase::{COINIT_APARTMENTTHREADED, COINIT_DISABLE_OLE1DDE};
    use winapi::um::shellapi::ShellExecuteW;
    use winapi::um::winuser::SW_SHOW;

    static OPEN: &[u16] = &[0x6f, 0x70, 0x65, 0x6e, 0x00];
    let file =
        U16CString::from_str(file).map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?;
    let code = unsafe {
        let _ = CoInitializeEx(
            ptr::null_mut(),
            COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE,
        );
        ShellExecuteW(
            ptr::null_mut(),
            OPEN.as_ptr(),
            file.as_ptr(),
            ptr::null(),
            ptr::null(),
            SW_SHOW,
        ) as usize
    };
    if code > 32 {
        Ok(())
    } else {
        Err(unimplemented!("code: {}", code))
    }
}

fn startfile_with_cmd(file: &str) -> io::Result<()> {
    let cmd = file.chars().fold("start ".to_owned(), |mut cmd, c| {
        cmd.push('^');
        cmd.push(c);
        cmd
    });
    info!("cmd = {:?}", cmd);
    let status = Command::new("cmd")
        .stdin(Stdio::null())
        .stdout(Stdio::null())
        .stderr(Stdio::null())
        .arg("/C")
        .arg(cmd)
        .status()?;
    if !status.success() {
        return Err(unimplemented!("status: {}", status));
    }
    Ok(())
}

fn startfile_with_powershell(file: &str) -> io::Result<()> {
    let (mut cmd, mut in_quotes) = ("Start-Process -FilePath ".to_owned(), false);
    file.chars().for_each(|c| match c {
        '"' if in_quotes => {
            cmd += "\"`\"";
            in_quotes = false;
        }
        '"' => cmd += "`\"",
        c if in_quotes => cmd.push(c),
        c => {
            cmd.push('"');
            cmd.push(c);
            in_quotes = true;
        }
    });
    if in_quotes {
        cmd.push('"');
    }
    info!("cmd = {:?}", cmd);
    let status = Command::new("powershell")
        .stdin(Stdio::null())
        .stdout(Stdio::null())
        .stderr(Stdio::null())
        .arg("-Command")
        .arg(cmd)
        .status()?;
    if !status.success() {
        return Err(unimplemented!("status: {}", status));
    }
    Ok(())
}

fn encode_if_url(file: &str) -> Cow<str> {
    use percent_encoding::{utf8_percent_encode, SIMPLE_ENCODE_SET};

    if file.contains("://") {
        Cow::from(utf8_percent_encode(file, SIMPLE_ENCODE_SET).to_string())
    } else {
        Cow::from(file)
    }
}

#[cfg(test)]
mod tests {
    static FILE: &str =
        "http://example.com/articles/non-ascii-text?q1=0&q2=0";

    #[test]
    fn test_startfile_with_winapi() {
        super::startfile_with_winapi(FILE).unwrap();
    }

    #[test]
    fn test_startfile_with_cmd() {
        super::startfile_with_cmd(FILE).unwrap();
    }

    #[test]
    fn test_startfile_with_poewrshell() {
        super::startfile_with_powershell(FILE).unwrap();
    }
}

@qryxip qryxip force-pushed the use-powershell-instead-of-cmd branch from 9294ddc to d2ff2af Compare December 17, 2018 08:20
@amodm
Copy link
Owner

amodm commented Dec 17, 2018

winapi does seem to be a better choice, and the unsafe code seems contained enough. Possible for you to modify the PR to use this?

My only recommendation would be to make the static OPEN call to be legible. This reddit thread has some answers.

@qryxip qryxip force-pushed the use-powershell-instead-of-cmd branch from d2ff2af to 81cb934 Compare December 17, 2018 15:07
@qryxip
Copy link
Contributor Author

qryxip commented Dec 17, 2018

Done.

@amodm amodm merged commit b0219e5 into amodm:master Dec 17, 2018
@amodm
Copy link
Owner

amodm commented Dec 18, 2018

Published 0.4.0 after fixing windows build

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.

2 participants