[PATCH] Final removal of lp_posix_pathnames() from the smbd server main code paths.

[Uri Simchoni]

On 03/24/2016 12:23 AM, Jeremy Allison wrote:
> 
> Just checked carefully - the strchr_m(temp,'/')
> check is only being done in the allow_broken_path
> case - and this is set from :
> 
> !conn->sconn->using_smb2
> 
> in both places this is called (in filename.c
> and trans2.c). Which essentially means it's
> a "if SMB1 only" flag.
> 
> If you like I could replace the strchr_m(temp,'/')
> back with an lp_posix_pathnames() call plus a
> comment explaining it as it'll never fire in the SMB2+ case.
> 
> I'd rather not though, as I really want to get
> away from supporting these old bugs if I can
> help it.
> 
> Let me know if you want that before giving your
> rb+.
> 
> Jeremy.
> 

One misleading thing is that pdp->posix_path is also affected by this
change of behavior and is exported to the caller, but then none makes
use of this field - seems like it can be removed.

Beyond that:

        if (*pathname == '/') {
                pdp->posix_path = true;
                sepchar = '/';
        } else {
                sepchar = '\\';
        }

        if (allow_broken_path && (*pathname != sepchar)) {

Now *pathname != sepchar only if *pathname is neither / nor \, so this
block stops doing its orginal function. Seems like if you don't want to
support old bugs, you can remove it entirely.

Additionally, sepchar is also used outside the "if broken" block

Finally, and I'm not familiar enough with the history of it all to tell
whether or not we want to support buggy clients. Windows XP is by
definition not buggy, no matter how oddly it behaves. I hear from time
to time remarks that Volker is still supporting Windows 95 or DOS :)

I don't know - maybe someone with better understanding of DFS can OK this.

I also think it all boils down to this:
- Sometimes we need to determine whether it's POSIX or not based on
handshake, or request details, i.e. not based on heuristics.
- For SMB2 there is no handshake yet
- For SMB1 there's lp_posix_pathnames(), which trickles into
req->posix_pathnames, which arms smb_fname->flags. So it's not like
we've totally gotten rid of lp_posix_pathnames, we've just "centralized
it" (it's like proper use vs improper use of singleton in C++).
- If there is not request, or fsp, or smb_fname around - we still need
lp_posix_pathnames().
- For SMB2 the case of "no request/fsp/smb_fnam" will have to be
considered based on the protocol details (e.g. if it's a one-time
negotiation as in SMB1, it can be recorded in the connection struct, if
it's per request, we shall have to get the request, etc.).

Sorry for what appears to be turning into yet another case of
bike-shedding. The original task was to review a patch-set that does not
change behavior. I cannot make the call on whether or not we can/should
change behavior.

Thanks,
Uri.