Skip to content

Make OCP\Security stricter#7900

Merged
MorrisJobke merged 1 commit into
masterfrom
strict_security
Jan 17, 2018
Merged

Make OCP\Security stricter#7900
MorrisJobke merged 1 commit into
masterfrom
strict_security

Conversation

@rullzer

@rullzer rullzer commented Jan 16, 2018

Copy link
Copy Markdown
Member
  • Add typehints
  • Add return types
  • Opcode opts from phpstorm
  • Made strict

Signed-off-by: Roeland Jago Douma roeland@famdouma.nl

juliusknorr
juliusknorr previously approved these changes Jan 16, 2018
@juliusknorr

Copy link
Copy Markdown
Member

@rullzer Tests are failing:

1) Test\Security\HasherTest::testVerify with data set #0 (null, 'asf32äà$$a.|3', false)
TypeError: Argument 1 passed to OC\Security\Hasher::verify() must be of the type string, null given, called in /drone/src/github.com/nextcloud/server/tests/lib/Security/HasherTest.php on line 115
/drone/src/github.com/nextcloud/server/lib/private/Security/Hasher.php:147
/drone/src/github.com/nextcloud/server/tests/lib/Security/HasherTest.php:115

2) Test\Security\HasherTest::testVerify with data set #1 (null, false, false)
TypeError: Argument 1 passed to OC\Security\Hasher::verify() must be of the type string, null given, called in /drone/src/github.com/nextcloud/server/tests/lib/Security/HasherTest.php on line 115
/drone/src/github.com/nextcloud/server/lib/private/Security/Hasher.php:147
/drone/src/github.com/nextcloud/server/tests/lib/Security/HasherTest.php:115

@juliusknorr juliusknorr added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jan 16, 2018
@juliusknorr juliusknorr dismissed their stale review January 16, 2018 20:29

Tests failing

@MorrisJobke

Copy link
Copy Markdown
Member

ref #7392

* Add typehints
* Add return types
* Opcode opts from phpstorm
* Made strict
* Fixed tests: No need to test bogus values anymore strict typing fixes
this

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@codecov

codecov Bot commented Jan 16, 2018

Copy link
Copy Markdown

Codecov Report

Merging #7900 into master will increase coverage by 3.93%.
The diff coverage is 50%.

@@             Coverage Diff              @@
##             master    #7900      +/-   ##
============================================
+ Coverage     30.81%   34.75%   +3.93%     
  Complexity    24930    24930              
============================================
  Files          1604     1604              
  Lines         94988    94988              
  Branches       1376     1376              
============================================
+ Hits          29271    33011    +3740     
+ Misses        65717    61977    -3740
Impacted Files Coverage Δ Complexity Δ
lib/public/Security/StringUtils.php 0% <0%> (ø) 1 <1> (ø) ⬇️
lib/private/Security/Crypto.php 11.76% <0%> (-85.3%) 9 <4> (ø)
lib/private/Security/Hasher.php 61.11% <75%> (-36.12%) 18 <16> (ø)
lib/private/App/AppStore/Version/Version.php 0% <0%> (-100%) 3% <0%> (ø)
core/Controller/ContactsMenuController.php 0% <0%> (-100%) 4% <0%> (ø)
...mments/tests/Unit/Controller/NotificationsTest.php 0% <0%> (-100%) 4% <0%> (ø)
lib/private/Hooks/LegacyEmitter.php 0% <0%> (-100%) 1% <0%> (ø)
apps/files_trashbin/appinfo/app.php 0% <0%> (-100%) 0% <0%> (ø)
settings/Activity/SecuritySetting.php 0% <0%> (-100%) 8% <0%> (ø)
...eware/Security/Exceptions/NotLoggedInException.php 0% <0%> (-100%) 1% <0%> (ø)
... and 833 more

@rullzer

rullzer commented Jan 16, 2018

Copy link
Copy Markdown
Member Author

Test should be fixed :)

@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 16, 2018
@MorrisJobke

Copy link
Copy Markdown
Member

@MorrisJobke MorrisJobke merged commit cb0dbfa into master Jan 17, 2018
@MorrisJobke MorrisJobke deleted the strict_security branch January 17, 2018 08:29
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 enhancement technical debt 🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants