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

Validation for new storage key location #30222

Merged
merged 1 commit into from
Jan 29, 2018
Merged

Conversation

sharidas
Copy link
Contributor

Extra validation required if the new storage
key location is being provided to the command.
When is_dir returns null it wasn't captured by
the command. And hence the command was executed
successfully. This change would help to prevent
such scenario. If is_dir returns null throw exception.

Signed-off-by: Sujith H [email protected]

Description

The new root directory wasn't checked for null. This change helps to validate against null

Related Issue

#27660

Motivation and Context

Validate new root against null.

How Has This Been Tested?

  • Create dir foo outside the owncloud folder
  • Ran the command below:
sujith@sujith-Inspiron-5567 ~/test/owncloud $ ./occ encryption:change-key-storage-root ../foo
Cannot load Xdebug - it was already loaded
Change key storage root from tester to ../foo
Start to move keys:

In ChangeKeyStorageRoot.php line 141:
                                                                                                   
  New root folder doesn't exist. Please create the folder or check the permissions and try again.  
                                                                                                   

encryption:change-key-storage-root [<newRoot>]

sujith@sujith-Inspiron-5567 ~/test/owncloud $

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@sharidas sharidas added this to the development milestone Jan 22, 2018
@sharidas sharidas self-assigned this Jan 22, 2018
@codecov
Copy link

codecov bot commented Jan 22, 2018

Codecov Report

Merging #30222 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #30222      +/-   ##
============================================
- Coverage     58.36%   58.35%   -0.01%     
- Complexity    18566    18567       +1     
============================================
  Files          1093     1093              
  Lines         63771    63772       +1     
============================================
- Hits          37218    37217       -1     
- Misses        26553    26555       +2
Impacted Files Coverage Δ Complexity Δ
drone/src/apps/files_trashbin/lib/Expiration.php 96.55% <0%> (-1.73%) 29% <0%> (ø)
drone/src/lib/private/Server.php 82.55% <0%> (-0.15%) 129% <0%> (ø)
...c/core/Command/Encryption/ChangeKeyStorageRoot.php 94.5% <0%> (+0.06%) 32% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd113a3...4802f09. Read the comment docs.

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

Extra validation required if the new storage
key location is being provided to the command.
When is_dir returns null it wasn't captured by
the command. And hence the command was executed
successfully. This change would help to prevent
such scenario. If is_dir returns null throw exception.

Signed-off-by: Sujith H <[email protected]>
@PVince81
Copy link
Contributor

@sharidas please backport

@phil-davis
Copy link
Contributor

Backport stable10 #30357

@lock
Copy link

lock bot commented Aug 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants