wxFileName::Rmdir() incorrectly follows symbolic links to directories and thus does not work correctly

[jdagresta]

On platforms where symbolic links are supported (e.g. Solaris, AIX, Linux), if you do wxFileDialog::Rmdir() on a directory that is a symbolic link to another directory, or you do wxFileDialog::Rmdir(wxPATH_RMDIR_RECURSIVE) on a directory that contains a symbolic link to another directory, the Rmdir() fails because it is "following" the directory symbolic link instead of just deleting the symbolic link itself.

The result can be disasterous, for example if the contained directory symbolic link points to a directory in the OS - it can result in the OS files in the linked directory being deleted (rather than just the symbolic link being deleted).

I've attached the source for a program (demo_rmdir_problem.cpp) that can be used to demonstrate the problem (on these platforms). It brings up a dialog with two buttons. The first button is used to create (set up) sample directories and directory symbolic links in /tmp. The second button is then used to do a wxFileName::Rmdir(0) and a second wxFileName::Rmdir(wxPATH_RMDIR_RECURSIVE) to demonstrate the problems that occur. The program writes messages to a text control to show what is happening. When the program is exited/closed, it will clean up the stuff it created in /tmp.

Attached are patches for three files to fix this problem: filenamecpp.patch (for src/common/filename.cpp), filenameh.patch (for include/wx/filename.h), and interface.patch (for interface/wx/filename.h).

Rather than get bogged down in very old discussions that have already occurred as to whether IsDir() should return true for symbolic links, I've taken the approach to just fix the problems in Rmdir().

There was no existing wxFileName::IsLink() method so I used some code in filectrlg.cpp dealing with symbolic links as a pattern to create this method, and then I used the new method to determine when to do something different in Rmdir().

[jdagresta]

Program to demonstrate the problems

[jdagresta]

Patch file for src/common/filename.cpp

[jdagresta]

Patch file for include/wx/filename.h

[jdagresta]

Patch file for interface/wx/filename.h

[vadz]

This is definitely pretty bad but we need to apply #14542 first, it contains a general solution to the same problem and it doesn't make much sense to have 2 of them.

[jdagresta]

OK, that makes sense.

I think the changes for #14542 will still not fix the problem where wxFileName::Rmdir() is passed a directory which is itself a symbolic link to another directory.

But it looks like it will fix the "indirect" problem where a real directory passed to Rmdir() contains a symbolic link to a directory, although I would think that a call to DontFollowLink() would have to be added to Rmdir().

In that first case I believe the wxFileName::Rmdir() will either still incorrectly follow the symbolic link by virtue of this code:

wxDir d(path);
if ( !d.IsOpened() )

return false;

or it will end up not deleting the top-level directory symbolic link if d.IsOpened() is false.

So I think something will still need to be changed in wxFileName::Rmdir().

[vadz]

When you call Rmdir() on a symlink to a directory you probably actually want to delete the directory and not the symlink. But just in case you don't, calling DontFollowLink() before doing it should work as expected -- i.e. result in an error because a symlink can't be rmdir'd. Or am I missing something here?

BTW, if you can test the patch from the other ticket, it would be helpful. If you do, please leave any notes about it there (e.g. "it works" or "it breaks XXX"). TIA!

[jdagresta]

The caller of Rmdir() may not know that the "directory" is actually a symlink to a directory, e.g. in a TreeCtrl and/or ListCtrl widget where the directory symlink matches the pattern of files being filtered out in the Tree/List display and the user hits DELETE on such an entry.

In that case, it would be the symlink that you would want deleted and not the directory being linked to (which could be anywhere, including somewhere in the "OS" that you would never want to implicitly delete).

Should the caller always be responsible for testing for a smylink before calling Rmdir()? Well, right now that's not possible because there's no IsLink() (or corresponding) method and adding that responsibility to the caller now implies a potentially large impact as far as searching all Rmdir() calls in all wxWidgets applications.

In my opinion Rmdir() should just be made to do the "right" thing, as in all other ways the directory symlink is (currently) treated as if it is a directory. As far as I can tell, nothing in the #14542 patch explicitly changes this default behavior.

I've already gone ahead and used my proposed changes in our version of wxWidgets to fix our application and patched it to our customers, since I cannot leave our application with the possibility that it will delete linked to files (that may be part of the "OS") and I need Rmdir(RECURSIVE) to work "correctly" whether it's the top-level Rmdir() path that's a symlink or it's a lower-level entry that's a symlink.

This bug was one that existed in MainWin that we used to build our application with before we moved to wxWidgets (and we never moved to the version of MainWin that had a fix because we switched) and I just happened to test it today with wxWidgets - I was disappointed that it was also broken in wxWidgets and had to rush fix it.

I'm not sure when or if I'll have time to try to apply the #14542 patch instead to our version of wxWidgets and test the Rmdir() calls in our application. In any case as I've noted, without further changes to the Rmdir() code itself I already know that it won't work for us (because our application needs Rmdir of a top-level directory symlink to remove the symlink and not generate an error).

[vadz]

(In #14542) I'm going to apply the DontFollowLink() part of this patch but I'm still not totally happy about the changes to {File,Dir,}Exists() so I won't change the static variants of these functions -- just update the member ones to honour the setting of m_dontFollowLinks.

The main reason I'm unhappy about the static ones is pretty stupid but here it is: naming. I don't think wxFSItem is a good name (meaning not for the enum itself, as nobody cares about it, but as the prefix for its elements) but I can't for the life of me come with anything better. So instead of not doing anything, I've decided to apply the important part of the patch right now and add the other one later, when somebody comes up with a good idea about how to call these flags.

[VZ]

(In #14542) (In [72680]) Add support for symlinks to wxFileName.

Allow to work with the symlinks themselves and not the file they reference by
calling the new wxFileName::DontFollowLink().

Update Unix wxDir implementation to not treat symlinks to directories as
directories, this ensures that we don't recurse into the directories outside
of the original parent accidentally.

Closes #14542.

[dghart]

(In #14542) Replying to vadz:

The main reason I'm unhappy about the static ones is pretty stupid but here it is: naming. I don't think wxFSItem is a good name (meaning not for the enum itself, as nobody cares about it, but as the prefix for its elements) but I can't for the life of me come with anything better. So instead of not doing anything, I've decided to apply the important part of the patch right now and add the other one later, when somebody comes up with a good idea about how to call these flags.

How about wxFileSystemFlags (or wxFSFlags if you prefer brevity)? Or 'Mode'?

[vadz]

(In #14542) Both are too generic IMHO, and we also already have wxFileSystemOpenFlags which is for (virtual) wxFileSystem and not (physical) wxFileName so it could be also a bit confusing. BTW, we also already have wxFileKind too.

So I guess it would need to be some new wxFileExistsFlags (or, actually, unnamed, because what do we need its name for?) enum with elements wxFILE_EXISTS_FILE (or REGULAR?), wxFILE_EXISTS_DIR, ... and wxFILE_EXISTS_NO_FOLLOW.

Any better suggestions?

[dghart]

(In #14542) Replying to vadz:

Both are too generic IMHO, and we also already have wxFileSystemOpenFlags which is for (virtual) wxFileSystem and not (physical) wxFileName so it could be also a bit confusing. BTW, we also already have wxFileKind too.

OK

So I guess it would need to be some new wxFileExistsFlags (or, actually, unnamed, because what do we need its name for?) enum with elements wxFILE_EXISTS_FILE (or REGULAR?), wxFILE_EXISTS_DIR, ... and wxFILE_EXISTS_NO_FOLLOW.

I'm not sure I understand. If we don't bother with the static functions, do we need an extra flag in the enum at all? Even if so, is there any need to change the names from the anonymous wxFileSystemObject_Foo?

I'm perfectly happy to skip the statics. I added the flag to them for completeness, and because it self-documents the need to consider symlinks; but it's not that much extra work for the user to create a wxFileName object and call DontFollowLink().

Any better suggestions?

I don't dare make the alternative suggestion of reverting to a bool parameter ;)

[vadz]

(In #14542) Replying to dghart:

I'm not sure I understand. If we don't bother with the static functions, do we need an extra flag in the enum at all?

No, we don't, but I do agree that being able to use this functionality with static functions would be useful/convenient/less surprising.

Any better suggestions?

I don't dare make the alternative suggestion of reverting to a bool parameter ;)

Believe it or not, but I did consider it. However I'd also like to add a test for symlink existence as this can be useful and I'd really prefer to do it by adding flags to Exists() rather than by adding yet another LinkExists() so flags would be better here.

And for now the best I can see is to allow

if ( wxFileName::Exists(wxFILE_EXISTS_FILE | wxFILE_EXISTS_NO_FOLLOW) )
  ...

as I wrote above. OTOH it's not without its problems neither as this would mean that the non-static Exists() shouldn't take the same flags because wxFILE_EXISTS_NO_FOLLOW could conflict with m_dontFollowLinks.

So finally perhaps it's indeed better to do nothing for now and maybe add LinkExists() etc (FIFOExists()? `SocketExists()?) later if anybody asks for it.

[dghart]

(In #14542) Replying to vadz:

Believe it or not, but I did consider it. However I'd also like to add a test for symlink existence as this can be useful and I'd really prefer to do it by adding flags to Exists() rather than by adding yet another LinkExists() so flags would be better here.

And for now the best I can see is to allow
{{{
#!cpp
if ( wxFileName::Exists(wxFILE_EXISTS_FILE | wxFILE_EXISTS_NO_FOLLOW) )
...
}}}

Ah, I understand now.

OTOH it's not without its problems neither as this would mean that the non-static Exists() shouldn't take the same flags because wxFILE_EXISTS_NO_FOLLOW could conflict with m_dontFollowLinks.

Using wxFILE_EXISTS_NO_FOLLOW in non-static Exists() sounds to me like a feature, and certainly not a problem. fn(wxFILE_EXISTS_DIR | wxFILE_EXISTS_NO_FOLLOW) would test if fn holds a real dir, not a symlink, without changing fn's default behaviour.

So finally perhaps it's indeed better to do nothing for now and maybe add LinkExists() etc (FIFOExists()? `SocketExists()?) later if anybody asks for it.

I'm happy to implement this, or similar, if you wish. I wonder though if it's worth adding extra functions (FIFOExists() etc) which will seldom be used, to the already function-heavy wxFileName. wxFilename::Exists(wxFILE_EXISTS_FLAGS flag = wxFILE_EXISTS_ANY) is less readable, but allows testing for multiple types.

[vadz]

(In #14542) I do prefer to have a single Exists(flags = ALL) rather than ThisExists() and ThatExists() and so on. If you think it's not a problem to have both NO_FOLLOW flag and DontFollow() method, this is what should probably be done.

[dghart]

(In #14542) Attached is a new patch, filename.diff, that implements the flags parameter. I've combined block and char devices in wxFILE_EXISTS_DEVICE, but they could easily be separated if you prefer.
I've also left a gap before wxFILE_EXISTS_ANY for future entries.

[VZ]

(In #14542) (In [72707]) Allow testing for existence of specific file types in wxFileName.

Add "flags" parameter to wxFileName::Exists() to allow testing for the
existing of files of specific type: not only regular or directory but also
symlink, device, FIFO or socket.

And also to pass wxFILE_EXISTS_NO_FOLLOW flag inhibiting following the
symlinks without using DontFollowLink().

Closes #14542.

[vadz]

We do have a way to test for the links in wxFileName now, so the only question is what should be actually done here.

I agree that Rmdir(wxPATH_RMDIR_RECURSIVE) shouldn't follow symlinks found during the traversal of the tree, this is too dangerous. But I don't know about calling Rmdir() on a symlink itself. I see 3 possibilities here:

1. Follow it.
2. Don't follow it and return an error.
3. Don't follow it but just remove the symlink itself.

Of those, (1) is backwards compatible and (2) is the safest one. (3) really doesn't make much sense to me but this is what the current patch implements so I mention it for completeness.

On balance, I think (2) would be the best as it would make recursive and non-recursive directory removal work the same. What do you think?

[dghart]

The wxPATH_RMDIR_RECURSIVE doc says: "Delete the specified directory and all the files and subdirectories in it recursively". This sounds to me like your item 3.

Why would a user want to remove recursively all the contents of a dir, except any symlinks?

[vadz]

We should definitely remove the symlinks encountered during the recursive traversal. But I don't think we should remove the symlink passed directly to Rmdir(). IOW I think we only need to change the iteration over subdirectories in wxFileName::Rmdir() to not follow the symlinks (but handle them as plain files and so delete them), but we should not change ::wxRmdir() itself to delete them as the current patch does.

[dghart]

Replying to vadz:

We should definitely remove the symlinks encountered during the recursive traversal. But I don't think we should remove the symlink passed directly to Rmdir(). IOW I think we only need to change the iteration over subdirectories in wxFileName::Rmdir() to not follow the symlinks (but handle them as plain files and so delete them), but we should not change ::wxRmdir() itself to delete them as the current patch does.

My apologies. I misunderstood you.

FWIW, I agree.

[jdagresta]

I'm still trying to look through and understand all the other changes (from multiple tickets and change sets) that have already been applied, but I do understand that the other changes already applied make my proposed patches obsolete.

I will chime in again with my opinion of what wxFileName::Rmdir() should do if it is being passed a path argument which is itself a symlink to a directory.

The caller of Rmdir() (and certainly the user running our application on UNIX that is like Windows Explorer) may not know that the "directory" is actually a symlink to a directory, e.g. in a TreeCtrl and/or ListCtrl widget (in our application like Windows Explorer) where the directory symlink matches the pattern of files being filtered out in the Tree/List display and the user hits DELETE on such an entry.

In that case, it would be the symlink that the user would want deleted and not the directory being linked to (which could be anywhere, including somewhere in the "OS" that you would never want to implicitly delete). That's what the user would want to happen whether the symlink was at the top level or whether a symlink was encountered recursively.

Should the caller (i.e. in the application code) always be responsible for testing for a smylink before calling Rmdir()? That's what would have to be done if (2) is applied. But adding that responsibility to the caller would imply a potentially large impact to developers of wxWidgets applications as far as searching for all Rmdir() calls in all applications that might run into this situation.

And to me it seems inconsistent that you would treat a symlink encountered during the recursive traversal as "don't follow the symlink and just remove the symlink" but not treat a top-level symlink the same way.

So my vote is for (3).

Maybe there's a (4): Only if wxPATH_RMDIR_RECURSIVE is specified and a top-level symlink is specified then do (3), otherwise if RECURSIVE is not specified and you have a top-level symlink then do (2). But that may be too confusing.

If it's a problem that (3) introduces a change in behavior, then another option would be to add a new flag for Rmdir() that specifies what to do if the path argument is itself a symlink. But that would still mean that application developers would still have check for and make code changes and other changes that have already been applied (such as to wxDir) have already introduced changes in behavior.

Whatever is decided, I will encorporate the wxWidgets changes that are applied and then make any needed changes in our application code to make it do what it needs to do (e.g. adding a test for symlink before calling Rmdir and calling wxRemoveFile instead or Rmdir).

[vadz]

IMO (3) is illogical because you should handle a symlink to a directory either as a directory or as a (special kind of a) file. If you do the former, you should delete the directory contents too. If you do the latter, you shouldn't delete anything because calling wxRmdir() on plain file returns an error, so why should it delete a file that happens to be a symlink?

[jdagresta]

I accept your decision.

Am I right, then, that with the other applied changes to (UNIX) wxDir() that no further changes are needed to wxFileName()::Rmdir(wxPATH_RMDIR_RECURSIVE) for the (1) behavior and that Rmdir() will now correctly handle any symlinks that are found recursively (by just deleting the recursively found symlinks and not following the symlinks)?

If that is so, then as far as I'm concerned you can close this ticket. For our application it does not matter whether Rmdir(wxPATH_RMDIR_RECURSIVE) does (1) or (2), because I will need to change our application code such that if the path argument to Rmdir() is a symlink it calls wxRemoveFile() instead of Rmdir() (now that there is a way to test for symlinks due to other applied changes).

If you feel it is appropriate to change the Rmdir() behavior to (2), that is fine.

[jdagresta]

Well, I'm going to try one more time to argue that (3) is the "correct" behavior when doing wxFileName::Rmdir(wxPATH_RMDIR_RECURSIVE) with an argument which is a symlink to a directory.

There is no RECURSIVE mode of wxRmdir(), so on a UNIX system it would seem to me that the only equivalent to doing an "rm -r path" command would be wxFileName::Rmdir(). Now if one does an "rm -r path" command on UNIX where the "path" is a symlink to a directory, what does it do? It simply removes the "path" symlink without touching in any way the directory to which the symlink points.

I believe this argues to having Rmdir(RECURSIVE) behave as (3) on UNIX.

The fact that it currently behaves as (1) is probably due to the fact that the scenario of symlinks was not considered when it was written rather than by design.

If this argument doesn't sway you, then I will still accept your decision.

[vadz]

Replying to jdagresta:

Well, I'm going to try one more time to argue that (3) is the "correct" behavior when doing wxFileName::Rmdir(wxPATH_RMDIR_RECURSIVE) with an argument which is a symlink to a directory.

OK, I'm convinced. Just to be clear: wxRmdir(symlink) would still fail because it should behave the same as POSIX rmdir() which returns ENOTDIR for symlinks, but wxFileName::Rmdir(wxPATH_RMDIR_RECURSIVE) is indeed supposed to mimic rm -r and so should remove it.

If you could please update the patches to work like this with the current version (or maybe wait until David's changes to wxDir iteration first?), it would be great.

Thanks a lot in advance!

[dghart]

Replying to vadz:

If you could please update the patches to work like this with the current version (or maybe wait until David's changes to wxDir iteration first?), it would be great.

I may as well do this; it will follow easily from the way I'm about to alter wxDir.

[dghart]

(In #14542) As discussed in #14543, the attached wxdir.diff reverts the previous change, and instead makes following symlinks the default. There is a new flag in the wxDirFlags enum to control this.

This patch doesn't touch wxFileName::Rmdir (I'll do that separately). As a result, its use in the FileNameTestCase:929 causes looping; for about 20 seconds on my machine (I don't know what stops it). You may therefore wish to coordinate applying this and a fix for #14649.

[dghart]

The attached rmdir.diff, which depends on the latest #14542 patch, implements my understanding of the above discussion. In particular:

1) with the wxPATH_RMDIR_RECURSIVE flag, internal symlinks will themselves be deleted, not followed.
2) with the wxPATH_RMDIR_RECURSIVE flag, a passed symlink will be deleted, not followed.

The discussion avoided the following situations:

3) wxPATH_RMDIR_FULL, and not wxPATH_RMDIR_RECURSIVE. I've not-followed any symlinks, so unexpected deletions won't occur.
4) What about the situation where the passed 'dir' is a symlink and 1) no flag, 2) only wxPATH_RMDIR_FULL is passed? The patch ignores this situation; is that correct?

FileNameTestCase now succeeds again without any looping. I added a should-fail test for 4)

[jdagresta]

(In #14542) Note: there are two references in the latest wxdir.diff patch just attached to "Note that wxDIR_NO_FOLLOW is relevant only on Linux".

Shouldn't that be a reference to "... only on UNIX" or "... only on UNIX/Linux"?

It applies to other flavors of UNIX than Linux, such as Solaris, AIX, etc., namely any OS that supports symlinks.

[dghart]

(In #14542) Replying to jdagresta:

Shouldn't that be a reference to "... only on UNIX" or "... only on UNIX/Linux"?

Fair enough.

[jdagresta]

I'm not convinced that the latest rmdir.diff patch will stil work correctly for the case where the dir passed to Rmdir() is itself a symlink and should just be removed without affecting anything in the linked-to directory.

Won't the symlink'd to top-level dir be opened by virue of wxDir d(path) and still iterated through deleting any files and directories (whether the iterated files are symlinks or not) - although not recursively following any symlinks?

Or does adding the wxDirNoFollow flag to d.GetFirst() mean that no entry will be matched on wxDIR_DIRS or wxDIR_FILES if it is a symlink? But what about those entries in the top-level symlinked-to directory that are not themselves symlinks? Won't they still get deleted?

One reason that I'm not convinced is that my original proposal (filenamecpp.patch) had an added test for the top-level symlink right at the test (for UNIX) for flags !=0 so that it would not go into the flags != 0 code at all and just fall through to the code that checks to do wxRemoveFile() instead of wxRmdir().

It's not obvious to me that the latest proposed changes with wxdir.diff on #14542, in addition to earlier changes already applied, mean that no "normal" entry in a top-level symlinked-to directory will be matched on wxDIR_DIRS or wxDIR_FILES. That's by looking at the logic, even if wxdir.diff is applied, in the wxDirData::Read(wxString *filename) routine (in src/unix/dir.cpp).

I'm not sure, but it seems like "normal" directories and files inside the top-level symlinked-to directory will still get deleted, while either directory or file symlinks also will still get deleted.

Did you try a test of Rmdir() with a top-level symlink to a directory that itself contains both "normal" files and sub-directories as well as symlinks to files and directories and verify that none of the sub-directories or files or symlinks in the linked-to directory were deleted?

I could easily be wrong, but thought it was worth asking.

[vadz]

Replying to dghart:

The discussion avoided the following situations:
3) wxPATH_RMDIR_FULL, and not wxPATH_RMDIR_RECURSIVE. I've not-followed any symlinks, so unexpected deletions won't occur.

I think this is correct, FULL means to delete all the empty subdirectories (recursively), but do nothing with the files.

4) What about the situation where the passed 'dir' is a symlink and 1) no flag, 2) only wxPATH_RMDIR_FULL is passed? The patch ignores this situation; is that correct?

I think it's correct too because ::wxRmdir() should already fail with ENOTDIR error in this case, which is the expected behaviour.

FileNameTestCase now succeeds again without any looping.

I'm rather confused by this looping problem, I'll need to look into it because I really have no idea where would this be coming from. Do you have any more information about it by chance?

[vadz]

Replying to jdagresta:

I'm not convinced that the latest rmdir.diff patch will stil work correctly for the case where the dir passed to Rmdir() is itself a symlink and should just be removed without affecting anything in the linked-to directory.

The best way to answer this would be to add a test for this for the unit test, if it doesn't already check for it.

[vadz]

(In #14542) Replying to dghart:

This patch doesn't touch wxFileName::Rmdir (I'll do that separately). As a result, its use in the FileNameTestCase:929 causes looping; for about 20 seconds on my machine (I don't know what stops it). You may therefore wish to coordinate applying this and a fix for #14649.

FWIW I don't see any problems like this here...

[VZ]

(In #14542) (In [72739]) Change the way directory iteration flags are constructed.

Instead of explicitly constructing the flags from the flags that should be
included, construct them by excluding the flags that shouldn't be used. This
makes the code more stable in the sense that it will continue to work when new
flags, such as the upcoming wxDIR_NO_FOLLOW, are added.

See #14542.

[VZ]

(In #14542) (In [72740]) Add wxDIR_NO_FOLLOW flag for wxDir iteration.

This flag allows to avoid following the symbolic links during the directory
traversal. In particular, this means that links to the directories
(potentially outside the directory being traversed) are not considered as
directories at all when it is used, potentially avoiding surprises.

Closes #14542.

[VZ]

(In #14542) (In [72741]) Mention wxFILE_EXISTS_NO_FOLLOW in wxFILE_EXISTS_SYMLINK description.

Using wxFILE_EXISTS_SYMLINK without wxFILE_EXISTS_NO_FOLLOW can only be
fruitless, so mention that they should normally be used together in the
documentation.

An alternative solution would be to always add wxFILE_EXISTS_NO_FOLLOW
automatically if wxFILE_EXISTS_SYMLINK is used, perhaps we should do this
instead.

See #14542.

[VZ]

(In [72742]) Don't follow symlinks in wxFileName::Rmdir(wxPATH_RMDIR_RECURSIVE).

Following symlinks, possibly leading outside of the directory being removed,
is at best surprising and at worst dangerous, so don't do it and just mimic
the behaviour of "rm -rf", i.e. remove everything inside this directory,
including the symlinks themselves, but don't follow them.

Closes #14649.

[dghart]

Replying to jdagresta:

I'm not convinced that the latest rmdir.diff patch will stil work correctly for the case where the dir passed to Rmdir() is itself a symlink and should just be removed without affecting anything in the linked-to directory.
// snip
I could easily be wrong, but thought it was worth asking.

No, you were right. However Vadim has corrected the mistake, and I confirm that deleting symlinks is now 'safe'.

Replying to vadz:

I'm rather confused by this looping problem, I'll need to look into it because I really have no idea where would this be coming from. Do you have any more information about it by chance?

It happened (now cured) because wxFileName::Rmdir was following symlinks; in this case the symlink pointed to its parent dir, so Rmdir was called on the link multiple times. Presumably the loop was not infinite because the ELOOP limit was eventually reached.

[dghart]

(In #14542) Replying to VZ:

Using wxFILE_EXISTS_SYMLINK without wxFILE_EXISTS_NO_FOLLOW can only be
fruitless, so mention that they should normally be used together in the
documentation.

An alternative solution would be to always add wxFILE_EXISTS_NO_FOLLOW
automatically if wxFILE_EXISTS_SYMLINK is used, perhaps we should do this
instead.

Yes, I noticed this yesterday and was going to ask about it once the dust had settled.

I can't think of a reason to want wxFILE_EXISTS_SYMLINK without wxFILE_EXISTS_NO_FOLLOW. I'll create a patch.