Skip to content

Commit

Permalink
Don’t core dump when given invalid UTF-8 arguments
Browse files Browse the repository at this point in the history
By parsing OsStrings rather than Strings, it’s the getopts crate that’s doing the UTF-8 checking rather than us, so if one of them isn’t valid, it’ll just fail to parse rather than crash exa.

Also, save a few allocations here and there.
  • Loading branch information
ogham committed May 18, 2017
1 parent 2f79b4d commit de60b95
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 7 deletions.
6 changes: 3 additions & 3 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
extern crate exa;
use exa::Exa;

use std::env::args;
use std::env::args_os;
use std::io::{stdout, stderr, Write, ErrorKind};
use std::process::exit;

fn main() {
let args: Vec<String> = args().skip(1).collect();
let args = args_os().skip(1);
let mut stdout = stdout();

match Exa::new(&args, &mut stdout) {
match Exa::new(args, &mut stdout) {
Ok(mut exa) => {
match exa.run() {
Ok(exit_status) => exit(exit_status),
Expand Down
4 changes: 2 additions & 2 deletions src/exa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ pub struct Exa<'w, W: Write + 'w> {
}

impl<'w, W: Write + 'w> Exa<'w, W> {
pub fn new<S>(args: &[S], writer: &'w mut W) -> Result<Exa<'w, W>, Misfire>
where S: AsRef<OsStr> {
pub fn new<C>(args: C, writer: &'w mut W) -> Result<Exa<'w, W>, Misfire>
where C: IntoIterator, C::Item: AsRef<OsStr> {
Options::getopts(args).map(move |(options, args)| {
Exa { options, writer, args }
})
Expand Down
10 changes: 8 additions & 2 deletions src/options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,16 @@ pub struct Options {

impl Options {

// Even though the arguments go in as OsStrings, they come out
// as Strings. Invalid UTF-8 won’t be parsed, but it won’t make
// exa core dump either.
//
// https://github.com/rust-lang-nursery/getopts/pull/29

/// Call getopts on the given slice of command-line strings.
#[allow(unused_results)]
pub fn getopts<S>(args: &[S]) -> Result<(Options, Vec<String>), Misfire>
where S: AsRef<OsStr> {
pub fn getopts<C>(args: C) -> Result<(Options, Vec<String>), Misfire>
where C: IntoIterator, C::Item: AsRef<OsStr> {
let mut opts = getopts::Options::new();

opts.optflag("v", "version", "show version of exa");
Expand Down
4 changes: 4 additions & 0 deletions xtests/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ COLUMNS=80 $exa $testcases/file-names -R 2>&1 | diff -q - $results/file_names_R
$exa $testcases/file-names -1 2>&1 | diff -q - $results/file_names_1 || exit 1
$exa $testcases/file-names -T 2>&1 | diff -q - $results/file_names_T || exit 1

# At least make sure it handles invalid UTF-8 arguments without crashing
$exa $testcases/file-names/* 2>/dev/null


# File types
$exa $testcases/file-names-exts -1 2>&1 | diff -q - $results/file-names-exts || exit 1
$exa $testcases/specials -l 2>&1 | diff -q - $results/specials || exit 1
Expand Down

0 comments on commit de60b95

Please sign in to comment.