tuftool: support filepaths in addition to file:// urls

[webern]

Maybe we should detect the absence of a URL scheme in tuftool arguments such as --targets-url. Currently we require a file:// URL for a local directory, but maybe we could make it more convenience and detect when a filepath lacks a protocol scheme, thus treating it as a path?

Unfortunately this would probably require renaming such arguments, otherwise --targets-url could take a path, which might be surprising.

This is related to #347, where requiring Windows users to convert C:\my\path to file:///C/my/path could be inconvenient.

Open for discussion...

[Cytro54]

IMO, we should support raw paths under windows; after all, they are already supported under *nix, but I'll admit, this is a complex problem.

[Cytro54]

Maybe we could detect when no protocol is found, (like raw paths) and convert them to file:// paths without changing the rest of the code, I mean, for the rest of the tool, it would be just like a file:// URL was given, and the execution could go on.

Plus, it looks like (but don't take my word for it, I'm just coming back right now to finish windows support), the only windows-unfriendly module is the keySource module. I don't remember having problems with the other ones.

I wrote a patch that, if the URL wasn't parsed successfully, tries to convert it into a file:// URL, if it fails, the URL is deemed as unrecognized, if it succeeds, it recursively calls parse_keysource() with the edited URL.

EDIT : It looks like this patch passes tests, If you want to ,I can create a new PR.

[webern]

I think this can be done without any manual parsing by using the existing parsing of Url and PathBuf types. I was thinking of something like

Pseudocode:

pub(crate) enum PathOrUrl {
    Path(PathBuf),
    Url(Url),
}

impl From<&str> for PathOrUrl {
    fn from_str(s: &str) -> Result<Self> {
        // first try PathBuf::from_str(s)
        // if it works return PathOrUrl::Path(path)
        // if it doesn't work try Url::from_str(s) and return PathOrUrl::Url(url)
    }
}

impl PathOrUrl {
    pub(crate) fn as_url() -> Url {
        // TODO - return a Url
    }
}

This type could be used in the StructOpt arguments where we currently use Url (or do we use String currently?)

[webern]

after all, they are already supported under *nix

Are they? I would expect an error with:

--metadata-url /my/path/

I thought that we are currently required to do

--metadata-url file:///my/path/

[tjkirch]

How about a --targets-path argument that assumes a local path, in addition to having --targets-url? It'd surprise me to assume a path. Or if we really want it to assume paths, could it try to check existence of the path first, and also check if it parses as a domain (so you can similarly omit https://)?

[webern]

How about a --targets-path argument that assumes a local path, in addition to having --targets-url? It'd surprise me to assume a path. Or if we really want it to assume paths, could it try to check existence of the path first, and also check if it parses as a domain (so you can similarly omit https://)?

Yes, that could work. I guess we would error if the user provided both --targets-path and --targets-url, but it has the advantage of not a) having a weird argument name, or b) a breaking change to change an argument name.

[Cytro54]

Are they? I would expect an error with:

--metadata-url /my/path/

I think that this will return an error, only valid URLs are supported here, and it doesn't have to do w/ windows compatibility.

What's causing problems is the --key argument, in the tuftool create subcommand. Because the KeySource module uses a different method to parse user input (so that it can support aws-ssm:// and aws-kms:// URLs), bare paths works on *nix but not on Windows (Maybe because of the different path conventions between the two platforms)

I can think of 2 solutions to tackle this problem:

• Trying to detect bare paths only in the --key argument under *nix and windows
• Stop supporting bare paths in this argument and fixing tests (they call the binary w/ bare paths)

[cbgbt]

Trying to detect bare paths only in the --key argument under *nix and windows
In #660 I took this approach to the --key argument by first checking to see if the argument was a file that exists.

That change doesn't solve the issue around specifying targets. fwiw I like @tjkirch's suggestion.