Issue 11 Use filepath.Join to avoid hardcoded path separators

[jacobalberty]

This should solve #11 . I did not change example_test.go however since /opt/GoCA/CA as a path is a pretty heavy unixism to begin with and would need more than just filepath.Join to fix.

I also did not change the hardcoded check in caPathInit for ".//" as that seems to be for testing not normal operation.

I did try to extend the storage functions to take variadic parameters where possible so you can for example call LoadFile("dir1", "dir2", "file.pem") instead of LoadFile(filepath.Join("dir1", "dir2", "file.pem"))

Something I did notice however is there is a lot of use of os.Stat(); os.IsNotExist() for example in CheckCertExists This is a bit of an anti pattern it would be better to attempt to open the file and check os.IsNotExist if opening the file returns an error.

[necheffa]

@jacobalberty I think the Unix specific path behavior in the test suite, both in caPathInit() and goca_test.go will be a separate issue since that is broader scope than what was originally described in #11 although meets a higher level intent to have Windows compatibility. Good catch.

Could you elaborate on os.Stat(); os.IsNotExist() vs opening the file first and then checking os.IsNotExist? Is the concern a perf hit accessing the disk twice when once is sufficient to do the needed checks? Or, something else?

[jacobalberty]

@necheffa on the os.Stat() thing. It's a race condition. It's entirely possible for it to use os.Stat() and then the file get removed, or created before the results of os.Stat are used.

It's better to use os.OpenFile() and use the error code of that to determine if the file exists, you can use os.O_RDWR|os.O_CREATE|os.O_EXCL as the flags for that call to create the file if it doesnt exist but you want it to. This change would require adding a file handle to the File struct and using that file handle in calls like LoadFile. This would be a much bigger lift than just moving to filepath.Join though, it's just something I noticed that could be done to make the code more robust in the future.

[necheffa]

@jacobalberty I see what you mean. Although, I don't think skipping os.Stat() totally fixes us. If there is a race between os.Stat() and os.IsNotExist(), there are just races in all the public facing storage routines generally.

Another good catch; I'll submit another issue to track this.