Created attachment 3768 [details]
Adds functions to enable posix extensions

Here's a first cut of something to do this, against GIT MASTER from 2008-11-29. Compiles ok, but haven't tested it out yet. The SMBC_enable_posix function is pretty much just a copy+paste of the cmd_posix function from client/client.c, but with the cmd_posix still left intact (due to it needing to set CLI_DIRSEP_CHAR/STR and client_set_cur_dir, but only if the server has long filename support, and this being hard to tell from outside SMBC_enable_posix). Should *hopefully* work though, even though it's a tad inelegant right now...

Looks like a nice enhancement.  Would you please do the following few things:

1. It looks like everything from d_printf to d_printf in SMBC_enable_posix is all debug code.  Please wrap that whole block of code (including the d_printfs) inside of:

  if(DEBUGLVL(2)) 

and uncomment the d_printf calls.

2. In libsmb_context.c where you set the default value, set it to False instead of True.  This both makes this a very low danger patch since the new code is disabled by default, and also leaves the default at its previous value for backward compatibility.

3. Test to ensure the patch works as you intend, and that it doesn't break previous functionality.

4. Commit the entire patch to your local git tree with a meaningful commit message, and then use this command to generate the diff with a Signed-off-by header:

  git-format-patch -s --stdout > somefile.patch

5. Attach somefile.patch to this bug report and I can then apply your patch to the master tree.

Thanks!

Derrell

The problem with this is it makes the get/set capabilities call on every open. This is massively inefficient. The patch needs changing to call :

+		if (context->internal->posix_extensions)
+			SMBC_enable_posix(context, targetcli);
                 
inside SMBC_server(), just before the check :

        if (context->internal->smb_encryption_level) {

at libsmb/libsmb_server.c:477

Jeremy.

Arg. My bad - needs to be done *after* the encryption setup at after line 500  inside SMBC_server(), libsmb/libsmb_server.c.

Jeremy.

Created attachment 3773 [details]
Updated patch

(In reply to comment #4)
> Arg. My bad - needs to be done *after* the encryption setup at after line 500 
> inside SMBC_server(), libsmb/libsmb_server.c.

I'd noticed things like that right after I actually tried testing it out... I've just attached my latest working version of the patch. Note that a) it's still buggy b) has various extra debug added on and c) still enables the posix extensions by default. I still need to do some more work on it before it'll get to a fully usable state (which might not happen before the weekend) but figured I might as well update people on where I was at the moment.

Derrell, I'd love to get this into 3.3 - what is the current status on ABI compatibility and major/minor version number changes for libsmbclient and 3.3 ?
Jeremy.

Jeremy, I don't believe I've made any non-backward-compatible changes any time recently that would force a version bump.  This change adds functionality to the get/setOptions functions, but no new function entry points.  I don't believe this requires a version bump as long as we're allowed to add functionality without a version bump.

Derrell

I thought we bumped the minor version number when we added a function. That way if someone builds against a later version that might use the new function they know their code won't link against an earlier version. But we haven't finalized the minor version number for 3.3 yet, so as long as this code is finished, and the minor version number for 3.3.0 is greater than 3.2.x, then we should be ok.
Jeremy.

Is there any chance to get this code into libsmbclient? The lack of POSIX extensions in libsmbclient means that using libsmbclient to browse Unix/Linux servers ends up with mangled filenames.

Yeah, should be able to get this in for 3.6.0. Ping me if you don't see it go in next week.

Jeremy.

(In reply to comment #10)
> Yeah, should be able to get this in for 3.6.0. Ping me if you don't see it go
> in next week.

I don't think I saw anything go in...

Jeremy, do you have an updates on this?

It looks like the source still has no trace of your patch: https://git.samba.org/?p=samba.git;a=blob;f=source3/libsmb/libsmb_server.c;hb=HEAD

Taking ownership to see if I can get this fixed finally..

Jeremy.

Jeremy, did this functionality ship without the bug being updated (as with bug 7393) ?

(In reply to Bastien Nocera from comment #15)

No, this never got integrated.

Created attachment 13563 [details]
Test program

9 years later, I'm prodding this again. This is a test program for the issue (heavily cribbed from https://lists.samba.org/archive/samba-technical/2002-November/025295.html)

So, I can't seem to reproduce the problem currently. All the filenames I get back from running this just seem to work fine.

I ran it as "gcc test2.c -I ./bin/default/include/public/ -Lbin/shared -lsmbclient -o test && LD_LIBRARY_PATH=bin/shared ./test -D "smb://192.168.1.2/media/video/youtube/" -U guest -P guest"
(replace path as appropriate for a local Samba machine)

(In reply to Tom Parker from comment #17)

Compiled with:
gcc test2.c `pkg-config --libs --cflags smbclient` -lsmbclient -o test

But auth_data_fn() is never called, so:
SPNEGO login failed: The attempted logon is invalid. This is either due to a bad username or authentication information.
Result : -1, errno=1[Operation not permitted]

I had to make a few changes locally, mainly using:
smbc_setFunctionAuthDataWithContext (context, auth_data_fn);
and adding SMBCCTX *context as the first arg to auth_data_fn and removing all the "smbc_init()" code.

Anyway, with libsmbclient 4.7.0:
Found entry type <8>, named <SCYLKB~N.PNG>
Found entry type <8>, named <S66JO8~S.PNG>
Found entry type <8>, named <SP9SR5~5.PNG>
Found entry type <8>, named <SGJSGU~M.PNG>
Found entry type <8>, named <SJL6HE~Y.PNG>
Found entry type <8>, named <SXKCK3~T.PNG>
Found entry type <8>, named <S64AIN~C.PNG>
Found entry type <8>, named <SSKJCY~J.PNG>
Found entry type <8>, named <S2G19J~E.PNG>

On the server:
$ ls *:*
Screenshot from 2012-07-13 10:26:32.png  Screenshot from 2012-08-16 17:34:12.png  Screenshot from 2012-08-28 16:33:32.png  Screenshot from 2012-10-08 14:06:44.png  Screenshot from 2014-04-23 17:56:55.png
Screenshot from 2012-08-16 17:33:31.png  Screenshot from 2012-08-28 11:59:17.png  Screenshot from 2012-10-04 16:44:59.png  Screenshot from 2014-04-23 14:58:26.png

I could also access those files with the expected names containing ":" using cifs and option "vers=1.0".

The test server is an updated Synology NAS.

I also tried bumping the maximum allowed SMB version to 3.0, and passing vers=3.0 to cifs, same result (success), and same result with the test program (failure).

Were your tests with the patch applied by any chance? Did you have a rebased version of this 9 year old patch still applies?

Created attachment 13582 [details]
rebased patch

Git-formatted updated patch.

(In reply to Bastien Nocera from comment #20)

This patch will need more work, there wasn't enough context in some cases, which means it got applied to the wrong place, and there's also some code changes around the hunks that did apply.

Created attachment 13592 [details]
Fixed test program with context calls

Thanks, I've got a version of the test program here that uses those sorts of changes. It fails in the way you were seeing (now I realised I needed filenames with ':' in), even with the switch on. I get "Server doesn't support UNIX CIFS extensions" which means SERVER_HAS_UNIX_CIFS is returning false. This is slightly odd, as the same server  happily says it's got the Unix extensions enabled to smbclient...

Created attachment 13593 [details]
Revised patch, with

Current version of the patch I'm working with is attached. It's got all the fixes requested by earlier comments, except that it no longer works with against the current git master. I'm going to be away for the next 2 weeks, but will try and investigate further once I get back.

(In reply to Tom Parker from comment #23)

Did you get a chance to try this again?

It's taken a few years, but I've got a new MR for this at https://gitlab.com/samba-team/samba/-/merge_requests/2888

(In reply to Tom Parker from comment #25)

Thanks Tom!