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

Fixes Windows compilation #74

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

colindean
Copy link
Contributor

This works, although I'm not sure if it's the best way to do it.

Fixes #70

This works, although I'm not sure if it's the best way to do it.
I couldn't get the types to play nicely.
@sirwart
Copy link
Owner

sirwart commented Sep 14, 2023

Hmm, it looks like the Windows tests are failing for some reason. From looking at the test I don't see any obvious reasons why it should be failing when it does. Do you have a way to test locally? The only idea I have is that the test files have Unix line endings and not Windows.

@colindean
Copy link
Contributor Author

I'm seeing tests fail against 9ee96d6 and v0.1.7 on WSL. It's might be a line endings problem but it might be some other escaping problem. I've taken some swings at it but coming up short.

These fail in one_per_line:

aws azure random

These fail in one_per_file:

pem_key private_key

If skip these files, it passes tests.

$ file test/one_per_line/aws
test/one_per_line/aws: ASCII text, with CRLF line terminators

If I run dos2unix on the random file, it passes. Then I ran it on them all, and removed the skip list, and it passed.

I'll bet you can reproduce it locally if you ran unix2dos on the test files.

@colindean
Copy link
Contributor Author

Here's my kludgy modified test:

    #[test]
    fn no_false_negatives() {
        let skip_list = vec![
            /*"aws",
            "azure",
            //"random",
            "pem_key","private_key"*/];
        for maybe_entry in fs::read_dir("test/one_per_line").unwrap() {
            let entry = maybe_entry.unwrap();
            if skip_list.contains((&entry.path().file_name().unwrap().to_str().unwrap())) {
                continue
            }
            //let contents = fs::read_to_string(entry.path()).unwrap();
            //let actual = contents.matches(LINE_ENDING).count();
            let fh = File::open(entry.path()).unwrap();
            let con = BufReader::new(fh).lines();
            let ac = con.count();


            let res = find_secrets(
                &[entry.path()],
                &[],
                false,
                false,
                BufferWriter::stdout(ColorChoice::Never),
            );

            assert_eq!(
                res.unwrap(),
                ac,
                "{:?}",
                entry.file_name()
            );
        }
        for maybe_entry in fs::read_dir("test/one_per_file").unwrap() {
            let entry = maybe_entry.unwrap();
            if skip_list.contains((&entry.path().file_name().unwrap().to_str().unwrap())) {
                continue
            }
            let res = find_secrets(
                &[entry.path()],
                &[],
                false,
                false,
                BufferWriter::stdout(ColorChoice::Never),
            );
            assert_eq!(res.unwrap(), 1, "{:?}", entry.file_name());
        }
    }

@colindean
Copy link
Contributor Author

Found it.

Gotta add \r? to the RANDOM_STRING_REGEX

This is apparently required to enable tests to pass on a native Windows
clone of the source code, including in CI. Prior to this change, one
could run unix2dos on the test files to replicate the failure
I experienced on Windows even when tests pass on Linux for the v0.1.7
tag and fail on Windows.

I don't think I've fixed a Windows specific bug in anything in a decade!
This is my gaming rig, not programming, haha.
@colindean
Copy link
Contributor Author

After some more toggling, I'm able to reproduce the failure when pem_key is CRLF but I'm out of steam for tonight.

@colindean
Copy link
Contributor Author

I've got myself convinced that this is a bug in how ripsecrets or grep_searcher is handling CRLF and d3197e2 has changes that should facilitate debugging. I want to refactor the find_secrets function but I'm holding off until you get a chance to take a look and greenlight what is likely to be really pulling apart that function.

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.

Does not work in Windows
2 participants