Bug 5926 - POSIX extensions cannot be enabled under libsmbclient
Summary: POSIX extensions cannot be enabled under libsmbclient
Alias: None
Product: Samba 3.5
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 3.5.0
Hardware: Other Linux
: P3 normal
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
Depends on:
Reported: 2008-11-28 19:55 UTC by Tom Parker
Modified: 2023-01-16 11:05 UTC (History)
8 users (show)

See Also:

Adds functions to enable posix extensions (5.55 KB, patch)
2008-11-28 20:46 UTC, Tom Parker
no flags Details
Updated patch (8.30 KB, patch)
2008-12-02 19:15 UTC, Tom Parker
no flags Details
Test program (2.47 KB, text/x-csrc)
2017-09-09 17:45 UTC, Tom Parker
no flags Details
rebased patch (8.18 KB, patch)
2017-09-12 15:17 UTC, Bastien Nocera
no flags Details
Fixed test program with context calls (1.57 KB, text/x-csrc)
2017-09-13 14:17 UTC, Tom Parker
no flags Details
Revised patch, with (8.25 KB, patch)
2017-09-13 14:19 UTC, Tom Parker
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Parker 2008-11-28 19:55:59 UTC
It would be useful to be capable to enable a number of the POSIX extensions - primarily the long filename support, but others might be useful - for libsmbclient-using applications. One such case is http://bugzilla.gnome.org/show_bug.cgi?id=562029 where gvfs (using libsmbclient to provide access to smb shares) is unable to enable POSIX extensions, thus resulting in mangled filenames for a number of cases.

smbclient has a "posix" command that does the requisite action, but external apps cannot simply copy the code for that function as it requires access to a number of internal samba functions inaccessible to external apps. Providing a setter/getter interface to do this for libsmbclient users would be very useful!
Comment 1 Tom Parker 2008-11-28 20:46:01 UTC
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...
Comment 2 Derrell Lipman 2008-11-30 13:56:59 UTC
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:


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.


Comment 3 Jeremy Allison 2008-12-02 19:06:10 UTC
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

Comment 4 Jeremy Allison 2008-12-02 19:08:33 UTC
Arg. My bad - needs to be done *after* the encryption setup at after line 500  inside SMBC_server(), libsmb/libsmb_server.c.

Comment 5 Tom Parker 2008-12-02 19:15:53 UTC
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.
Comment 6 Jeremy Allison 2008-12-02 19:17:54 UTC
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 ?
Comment 7 Derrell Lipman 2008-12-02 19:51:24 UTC
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.

Comment 8 Jeremy Allison 2008-12-02 20:10:25 UTC
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.
Comment 9 Bastien Nocera 2010-09-04 12:01:31 UTC
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.
Comment 10 Jeremy Allison 2010-09-04 12:47:23 UTC
Yeah, should be able to get this in for 3.6.0. Ping me if you don't see it go in next week.

Comment 11 Bastien Nocera 2012-06-06 14:41:39 UTC
(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...
Comment 12 Bastien Nocera 2014-01-29 14:56:44 UTC
Jeremy, do you have an updates on this?
Comment 13 federico 2014-07-31 14:58:00 UTC
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
Comment 14 Jeremy Allison 2014-08-03 05:50:09 UTC
Taking ownership to see if I can get this fixed finally..

Comment 15 Bastien Nocera 2015-09-09 21:40:39 UTC
Jeremy, did this functionality ship without the bug being updated (as with bug 7393) ?
Comment 16 Jeremy Allison 2015-09-09 21:47:01 UTC
(In reply to Bastien Nocera from comment #15)

No, this never got integrated.
Comment 17 Tom Parker 2017-09-09 17:45:29 UTC
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://" -U guest -P guest"
(replace path as appropriate for a local Samba machine)
Comment 18 Bastien Nocera 2017-09-12 15:00:45 UTC
(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.
Comment 19 Bastien Nocera 2017-09-12 15:06:26 UTC
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?
Comment 20 Bastien Nocera 2017-09-12 15:17:25 UTC
Created attachment 13582 [details]
rebased patch

Git-formatted updated patch.
Comment 21 Bastien Nocera 2017-09-12 23:38:43 UTC
(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.
Comment 22 Tom Parker 2017-09-13 14:17:07 UTC
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...
Comment 23 Tom Parker 2017-09-13 14:19:45 UTC
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.
Comment 24 Bastien Nocera 2017-11-23 12:13:50 UTC
(In reply to Tom Parker from comment #23)

Did you get a chance to try this again?
Comment 25 Tom Parker 2023-01-16 10:42:08 UTC
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
Comment 26 Bastien Nocera 2023-01-16 11:05:47 UTC
(In reply to Tom Parker from comment #25)

Thanks Tom!