SMB External Storage not Working when using Failover Cluster

[leameiners]

Hello

We have run into a similar problem as issue #3789 when using a failover cluster for the file server since in this setup backwards 8.3 compatibility cannot be enabled on the shares.

We've managed to find a workaround modifying the source code so that it doesnt rely on the "allinfo" command via smbclient but on "dir", as the former does not work on shares without 8.3 compatibility.

I leave the code below for review. It'd be great if this fix was merged into the code base as it seems we are not the only ones with this issue.

====================================================================
FILE: apps/files_external/3rdparty/icewind/smb/src/Wrapped/Share.php

• Added function dirRoot:

    public function dirRoot() {
            $output = $this->execute('dir /');
            return $this->parser->parseRootDir($output, $path, function ($path) {
                    return $this->getAcls($path);
            });
    }
  

Comment: This function is a helper function that is meant to do the same as the dir function, but restricted to the case of listing the root directory. It could be unified with the dir function but doing so would require modifying the behaviour of this for this special case. This function uses parseRootDir which is explained below.

• Modified the stat function, as follows:

    public function stat($path) {
            // some windows server setups don't seem to like the allinfo command
            // use the dir command instead to get the file info where possible
            if ($path !== "" && $path !== "/") {
                    $parent = dirname($path);
                    $dir = $this->dir($parent);
                    $file = array_values(array_filter($dir, function (IFileInfo $info) use ($path) {
                            $infoPath = $info->getPath();
                            if ( substr( $infoPath, 0, 2 ) === "//") {
                                    $infoPath = substr($infoPath, 1);
                            }
                            return $infoPath === $path;
                    }));
                    if ($file) {
                            return $file[0];
                    }
            } else {
                    $dir = $this->dirRoot();
                    $file = array_values(array_filter($dir, function (IFileInfo $info) use ($path) {
                            return $info->getPath() === "/";
                    }));
                    if ($file) {
                            return $file[0];
                    }
            }
  
            $escapedPath = $this->escapePath($path);
            $output = $this->execute('allinfo ' . $escapedPath);
            // Windows and non Windows Fileserver may respond different
            // to the allinfo command for directories. If the result is a single
            // line = error line, redo it with a different allinfo parameter
            if ($escapedPath == '""' && count($output) < 2) {
                    $output = $this->execute('allinfo ' . '"."');
            }
            if (count($output) < 3) {
                    $this->parseOutput($output, $path);
            }
            $stat = $this->parser->parseStat($output);
            return new FileInfo($path, basename($path), $stat['size'], $stat['mtime'], $stat['mode'], function () use ($path) {
                    return $this->getAcls($path);
            });
    }
  

Comment:

• The main if was modified as follows:
  • There is a bug in the getPath function: when it is called for a file living in the / (root directory) it returns the filename preceeded with // when it should be returned with a single /, to be able to match it when calling array_filter; hence the substr workaround to handle this case
  • An else clause was added to handle executing "dir /" and getting the information from there instead of the command "allinfo /", as the latter fails when the server doesn't support 8.3 filename encoding (which is forcibly so when using a failover setup for example). It is here we use the dirRoot specific function and have hardcoded the base path of this case for it to match.

=====================================================================
FILE: apps/files_external/3rdparty/icewind/smb/src/Wrapped/Parser.php

• Added function parseRootDir

    public function parseRootDir($output, $basePath, callable $aclCallback) {
            //last line is used space
            array_pop($output);
            $regex = '/^\s*(\.)\s\s\s\s+(?:([NDHARS]*)\s+)?([0-9]+)\s+(.*)$/';
            //2 spaces, . dot directory, optional type, size, date
            $content = [];
            foreach ($output as $line) {
                    if (preg_match($regex, $line, $matches)) {
                            list(, $name, $mode, $size, $time) = $matches;
                            if ($name == '.') {
                                    $mode = $this->parseMode($mode);
                                    $time = strtotime($time . ' ' . $this->timeZone);
                                    $path = '/';
                                    $content[] = new FileInfo($path, $name, $size, $time, $mode, function () use ($aclCallback, $path) {
                                            return $aclCallback($path);
                                    });
                            }
                    }
            }
            return $content;
    }
  

Comment:

• This function is similar to parseDir except for:
  • modified regex to find the "." dot directory (as this is the root)
  • modified if accordingly
  • and hardcoded return path to / as the "." is current and not root directory.

cheers,
Leandro.-

[szaimen]

Hello, would you be able to create a PR with your changes? Thank you! :)

[ghost]

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.