Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle exceptions in SMB::stat #7556

Merged
merged 1 commit into from
Dec 18, 2017
Merged

handle exceptions in SMB::stat #7556

merged 1 commit into from
Dec 18, 2017

Conversation

icewind1991
Copy link
Member

Properly handle non existing/non readable files

Signed-off-by: Robin Appelman <robin@icewind.nl>
@codecov
Copy link

codecov bot commented Dec 18, 2017

Codecov Report

Merging #7556 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #7556      +/-   ##
============================================
- Coverage     51.18%   51.18%   -0.01%     
- Complexity    24874    24876       +2     
============================================
  Files          1601     1601              
  Lines         94707    94712       +5     
  Branches       1368     1368              
============================================
  Hits          48475    48475              
- Misses        46232    46237       +5
Impacted Files Coverage Δ Complexity Δ
apps/files_external/lib/Lib/Storage/SMB.php 5.53% <0%> (-0.11%) 118 <5> (+2)

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds sane

@MorrisJobke MorrisJobke merged commit b01d20c into master Dec 18, 2017
@MorrisJobke MorrisJobke deleted the smb-stat-exception branch December 18, 2017 21:31
@MorrisJobke MorrisJobke mentioned this pull request Jan 2, 2018
30 tasks
@rik26
Copy link

rik26 commented Jun 5, 2019

With Nextcloud 16, the problem still appears with symlink on share, i tested the following code, and it works :
I put a comment here too :
#7332

/**
         * get the best guess for the modification time of the share
         *
         * @return int
         */
        private function shareMTime() {
                $highestMTime = 0;
                $files = $this->share->dir($this->root);
                foreach ($files as $fileInfo) {
                        try {
                                if ($fileInfo->getMTime() > $highestMTime) {
                                        $highestMTime = $fileInfo->getMTime();
                                }
                        } catch (NotFoundException $e) {
                                // Ignore this, can happen on unavailable DFS shares
-                         }
+                        } catch (ForbiddenException $e) {
+                               // Ignore this too : symlink
+                        }
                }
                return $highestMTime;
        }

@icewind1991
Copy link
Member Author

@rik26 can you submit a PR with your change

@rik26
Copy link

rik26 commented Jun 5, 2019

@icewind1991 It's not i'm not willing to help, but i don't understand how to make a PR... And sorry for the duplicate but as the issue was closed i created a new one ...

#15876

Sorry for being useless..

@MorrisJobke
Copy link
Member

Sorry for being useless..

You are not useless. Thanks for the hint already.

I opened a PR at #16195

rullzer pushed a commit that referenced this pull request Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants