Mimetype list integrity check should not fail if it's changed #15810
Conversation
Signed-off-by: Xheni Myrtaj <[email protected]>
Signed-off-by: Xheni Myrtaj <[email protected]>
Signed-off-by: Xheni Myrtaj <[email protected]>
Signed-off-by: Xheni Myrtaj <[email protected]>
Signed-off-by: Xheni Myrtaj <[email protected]>
xh3n1
force-pushed
the
mimetype-integrity-check
branch
from
af34cca to
5fc0477
Compare
Signed-off-by: Xheni Myrtaj <[email protected]>
| */ | ||
| public function generateFile(array $aliases): string { | ||
| // Remove comments | ||
| $keys = array_filter(array_keys($aliases), function($k) { |
There was a problem hiding this comment.
I think we can improve this a bit. It's possible to pass a flag ARRAY_FILTER_USE_KEY to array_filter. We can omit the foreach-unset below and call the array_filter on $aliases.
There was a problem hiding this comment.
As this is code copied over from the old location it's fine to leave it as it is now in this PR and change the code in a separate PR to separate concerns properly. But still a nice finding 👍
Do you mind to add them here as well? Regarding executing unit tests locally typically |
|
@MorrisJobke Thanks, I had to downgrade my PHPUnit version to make it work and I will push my test later. |
Co-Authored-By: Morris Jobke <[email protected]>
Ah ... right. We only run on version 5 or 6 and not on the latest version. |
xh3n1
force-pushed
the
mimetype-integrity-check
branch
from
e5608d9 to
1d434cb
Compare
|
Let me rebase to trigger CI again here. |
MorrisJobke
force-pushed
the
mimetype-integrity-check
branch
from
1d434cb to
46ffdc4
Compare
I just noticed that the tests are not yet included. Mind to push them? Feel free to overwrite my push, because I just rebased on master and it went through without conflicts. |
xh3n1
force-pushed
the
mimetype-integrity-check
branch
from
46ffdc4 to
72bb035
Compare
MorrisJobke
added
3. to review
xh3n1
force-pushed
the
mimetype-integrity-check
branch
from
f73050d to
1d434cb
Compare
|
Welcome back @xh3n1 👋 :D Tests failures seems unrelated. |
skjnldsv
added
4. to release
Signed-off-by: Xheni Myrtaj <[email protected]>
Signed-off-by: Xheni Myrtaj <[email protected]>
|
Thanks @skjnldsv 😄 |
|
Failures still seems unrelated :) |
|
Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22 |
|
@xh3n1 Nice! Thanks for your help on this 👍 |
Issue
Integrity check failed if mimetype list was changed. The expected behaviour would be that the integrity check would still be passing.
Fixes #10411
Fix
The fix is aiming to still perform some kind of integrity check over the mimetype list. It would still fail for manually changed mimetype lists but will pass for any valid ones.
This is done by:
Regenerating the mimetype list with all mimetype aliases
If the genereated file is same as the one in the disk, then generate the sha512 of the mimetype list file with only default aliases since they are the only ones that are signed.
I have also added "application/internet-shortcut": "link" in resources/config/mimetypealiases.dist.json
Because it's had been manually added in the generated list https://github.com/nextcloud/server/pull/6297/files#diff-6bcc9a99f4c75054294c154bc8e2f490 but not when you generated it via maintenance:mimetype:update-js .
Test plan
Create a mimetypealiases.json file with custom mimetypes and save it in config dir.
Run ./occ maintenance:mimetype:update-js
And then ./occ integrity:check-core
With these changes it should work, without it the integrity check would fail.
PS. I have written the tests for this locally, but I having issues with running PHPUnit locally.