-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Only execute plain mimetype check for directories and do the fallback… #24459
Only execute plain mimetype check for directories and do the fallback… #24459
Conversation
… only for non-directories Ref #23096 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
/backport to stable20 |
/backport to stable19 |
return $this->executeStringCheck($operator, $value, $actualValue) || | ||
$this->executeStringCheck($operator, $value, $this->mimeTypeDetector->detectPath($this->path)); | ||
$plainMimetypeResult = $this->executeStringCheck($operator, $value, $actualValue); | ||
if ($actualValue === 'httpd/unix-directory') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without looking deeper into detectPath
method, it should be OK to or
this against $plainMimetypeResult
since it is returned anyway if true. Unless detectPath
must run for a reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This then would always lead to the check to be true
.
Check for !is MIMETYPE
means that either httpd/unix-directory
or application/octet-stream
does not match and therefore the check is useless, because it can't check for both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it short: detectPath
never returns the folder mimetype, because it uses the extension to determine the mime type and if there is none, it returns application/octet-stream
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks sane but I didn't test.
… only for non-directories
Introduced with #23096
Fixes nextcloud/files_accesscontrol#182
@juliushaertl I'm open for other approaches, but this seems to be useful here.