Fix different behaviour of OpenOptions between Windows en Linux

[pitdicker]

case #1:
A file is opened as read-only and with .truncate(true).
On Windows this fails with Error { repr: Os { code: 87, message: "The parameter is incorrect.\r\n" } }.
On Unix the behaviour is undefined.
Solution: also fail on Linux with Error { repr: Os { code: 22, message: "Invalid argument" } }
This is a breaking change, but only for code that relies on the current undefined behaviour.

case #2:
A file is opened with .append(true), but without .write(true).
On Windows it is possible to append to the file, on linux it fails.
Solution: let .append(true) imply .write(true) on linux.

case #3:
A file is opened without access options.
On Windows it fails at the first read or write.
On Linux this falls back to read-only mode.
Solution: I prefer to also fail on Linux, with "Invalid argument". But this is a breaking change...
It is also possible to add the same fallback on Windows.

I think that calling setting .create(true) with read-only access is also undefined behaviour.
Should we check for this also? OpenBSD might be a good candidate to test this.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[retep998]

It would be a nice if someone made a table of every possible flag combination and their current behavior on each platform to highlight the differences.

[alexcrichton]

I'm pretty ok with this because these corner cases have always been somewhat nebulous in their meaning and what happens. We'll also always want a way to be able to unconditionally pass arguments down (e.g. platform-specific extensions), so whatever you can do today you should be able to do eventually when opting in to the flags.

I agree it'd be nice to have a meaning for all combinations on all platforms, but I also think it's fine to not block this PR on that. Can you be sure to add a specific test case for all the cases mentioned here?

[pitdicker]

@retep998 This is my best effort for a table containing all possible flag combinations:

current behavior

read	write	append	result
false	false	false	Unix: fallback to read-only
			Windows: open succeeds, but any operation on the file fails
TRUE	false	false	open read-only
false	TRUE	false	open write-only
false	false	TRUE	Unix: open read-only
			Windows: open in append-mode
TRUE	TRUE	false	open in read/write-mode
TRUE	false	TRUE	Unix: open read-only
			Windows: open in read and append-mode
false	TRUE	TRUE	open in append-mode
TRUE	TRUE	TRUE	open in read and append-mode

new behavior

read	write	append	result
false	false	false	Unix: fail
			Windows: open succeeds, but any operation on the file fails
TRUE	false	false	open read-only
false	TRUE	false	open write-only
false	false	TRUE	open in append-mode
TRUE	TRUE	false	open in read/write-mode
TRUE	false	TRUE	open in read and append-mode
false	TRUE	TRUE	open in append-mode //(.write is unnecessary)//
TRUE	TRUE	TRUE	open in read and append-mode //(.write is unnecessary)//

current when using .truncate(true)

read	write	append	result
*	false	false	Unix: if file exists, undefined
			Linux: if file exists, truncated to 0 length
			Windows: fails
*	TRUE	false	if file exists, truncated to 0 length
*	TRUE	TRUE	Unix: if file exists, truncated to 0 length
			Windows: fails
*	false	TRUE	Unix: if file exists, undefined
			Linux: if file exists, truncated to 0 length
			Windows: fails

new when using .truncate(true)

read	write	append	result
*	false	false	fails
*	TRUE	false	if file exists, truncated to 0 length
*	*	TRUE	fails

current when using .create(true)

read	write	append	result
*	false	false	Unix: if file does not exist, undefined (?)
			Linux/Win: if file does not exist, created
*	TRUE	*	if file does not exist, created
*	false	TRUE	Unix: if file does not exist, undefined (?)
			Linux/Win: if file does not exist, created

I am not completely sure about .create. I'll try to make a VM with OpenBSD and run some tests...

However, I can't see much use in creating a file, if you don't have the right to write anything to it

proposed when using .create(true)

read	write	append	result
*	false	false	fail
*	TRUE	false	if file does not exist, created
*	*	TRUE	if file does not exist, created

[pitdicker]

@alexcrichton I have added tests that hopefully cover all cases.

Also, .create(true) seems to always work

[alexcrichton]

Thanks @pitdicker! This should come up in triage this week.

[alexcrichton]

OK, we talked about this in triage the other day, and we didn't reach a concrete conclusion just yet but our thoughts were:

• This seems like a good idea for a "high level" abstraction like OpenOptions. Lower level primitives probably don't want to deal with these kinds of differences but it seems safe to say that OpenOptions isn't that low-level.
• We need to be careful about how cases like .append(true).write(false) are handled if certain options imply other options.
• We'll probably always want to allow the ability to override these implications and defaults and go straight down to the system arguments. This sort of extension trait already exists for Windows (but is unstable) and may be useful for Unix as well. There are various corner cases (as discovered by @sfackler in Add OpenOptions::exclusive #27217) which normally don't make sense but do make sense in some weird situations.

Overall this probably also just needs an extra careful audit to make sure that basically every case is handled (and thanks for the table @pitdicker it's a great start). The libs team thinks this is ok to merge but may want some more thorough review before doing so.

[retep998]

Could we perhaps add a table like that to the Rust documentation for OpenOptions so that users of Rust can understand all the combinations?

[pitdicker]

This pull request touches things that can have a bit difficult interactions, and where it is hard to see if all corner cases are covered.

To help I am now trying to document everything involved.
This includes:

• how the options in rust relate to the flags passed to the system on Unix and Windows,
• what the exact behavior of those flags is, and so what the result of options in Rust is,
• how options interact with each other and platform-specific options,
• some possible extensions (like Add OpenOptions::exclusive #27217)

So please hold of reviewing some more.

[pitdicker]

Ok, I have finally finished documenting...
RFC

[pitdicker]

I think it is best if I close this pull request for now (I made a bit off a mess of my branch).

Also writing the rfc turned up some more things. I will make a new branch and implement the rfc, before a new pull request