Skip to content

Only cache the mimetype if the file exists - #17332

Merged
rullzer merged 1 commit into
masterfrom
bugfix/noid/mimetype-changes-on-renaming-a-folder
Oct 14, 2019
Merged

Only cache the mimetype if the file exists#17332
rullzer merged 1 commit into
masterfrom
bugfix/noid/mimetype-changes-on-renaming-a-folder

Conversation

@nickvergessen

@nickvergessennickvergessen commented Sep 30, 2019

Copy link
Copy Markdown
Member

Otherwise files access control will cache "application/octet-stream"
for all the target node on:
rename, move, copy and all other methods which create a new item

To check this:

  1. Add an automated tagging rule which tags on mimetype NOT "httpd/unix-directory"
  2. Add an access control rule which checks for any mimetype
  3. Create a folder and rename it, the folder should not be tagged, but it is

Signed-off-by: Joas Schilling coding@schilljs.com

@nickvergessennickvergessen added this to the Nextcloud 18 milestone Sep 30, 2019
@nickvergessen

nickvergessen commented Sep 30, 2019

Copy link
Copy Markdown
MemberAuthor

/backport to stable17

@nickvergessen

nickvergessen commented Sep 30, 2019

Copy link
Copy Markdown
MemberAuthor

/backport to stable16

@nickvergessen

nickvergessen commented Sep 30, 2019

Copy link
Copy Markdown
MemberAuthor

/backport to stable15

@kesselb

kesselb commented Sep 30, 2019

Copy link
Copy Markdown
Contributor

Sorry for hijacking this pr 🙈

2f5eac1 looks like a regression from b5f407e. Here is a first report: #17331

rullzer
rullzer approved these changes Sep 30, 2019
View reviewed changes

@rullzerrullzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should really get more tests for this code.

@nickvergessen

nickvergessen commented Sep 30, 2019

Copy link
Copy Markdown
MemberAuthor

Yeah, seems like i @kesselb So maybe we need to modify that code block a bit more

@nickvergessen

nickvergessen commented Sep 30, 2019

Copy link
Copy Markdown
MemberAuthor

Well actually the code from b5f407e was removed by the rework of the workflow thing @juliushaertl
Can you point me to the correct place?

@kesselb

kesselb commented Sep 30, 2019

Copy link
Copy Markdown
Contributor

Hmm. I'm not sure if you can reproduce this issue on master. For now files_accesscontrol is not compatible with the new workflow: nextcloud/files_accesscontrol#132

@juliusknorr

juliusknorr commented Oct 7, 2019

Copy link
Copy Markdown
Member

For the new WFE the file info is set here:

server/apps/workflowengine/lib/Check/TFileCheck.php

Line 61 in 72a7fe8

$this->storage = $subject->getStorage();

juliusknorr
juliusknorr approved these changes Oct 7, 2019
View reviewed changes

@juliusknorrjuliusknorr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Works as advertised 👏

@juliusknorr

juliusknorr commented Oct 7, 2019

Copy link
Copy Markdown
Member

/backport to stable17

@juliusknorr

juliusknorr commented Oct 7, 2019

Copy link
Copy Markdown
Member

/backport to stable16

@juliusknorr

juliusknorr commented Oct 7, 2019

Copy link
Copy Markdown
Member

/backport to stable15

rullzer
rullzer reviewed Oct 8, 2019
View reviewed changes
Comment threadapps/workflowengine/lib/Check/FileMimeType.php Outdated
nickvergessen
nickvergessen commented Oct 8, 2019
View reviewed changes
Comment threadapps/workflowengine/lib/Check/FileMimeType.php Outdated
@nickvergessen
Only cache the mimetype if the file exists
d4d3725
Otherwise files access control will cache "application/octet-stream"
for all the target node on:
rename, move, copy and all other methods which create a new item
To check this:
1. Add an automated tagging rule which tags on mimetype NOT "httpd/unix-directory"
2. Add an access control rule which checks for any mimetype
3. Create a folder and rename it, the folder should not be tagged, but it is
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessenforce-pushed the bugfix/noid/mimetype-changes-on-renaming-a-folder branch from 6290ebc to d4d3725CompareOctober 14, 2019 08:05
@nickvergessennickvergessen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 14, 2019
@rullzer
rullzer merged commit 1bc100a into masterOct 14, 2019
@rullzer
rullzer deleted the bugfix/noid/mimetype-changes-on-renaming-a-folder branch October 14, 2019 19:38
@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Oct 14, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Oct 14, 2019

Copy link
Copy Markdown

backport to stable17 in #17538

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Oct 14, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Oct 14, 2019

Copy link
Copy Markdown

backport to stable16 in #17539

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Oct 14, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Oct 14, 2019

Copy link
Copy Markdown

backport to stable15 in #17540

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzerrullzerrullzer approved these changes
@blizzzblizzzblizzz approved these changes
@juliusknorrjuliusknorrjuliusknorr approved these changes

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: workflows

Projects

None yet

Milestone

Nextcloud 18.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@nickvergessen@kesselb@juliusknorr@rullzer@blizzz