2/3 of the hooks of OC\Files\Storage\Shared can point to unexisting files #23620

Closed
oparoz opened this Issue Mar 29, 2016 · 14 comments

Projects

None yet

6 participants

@oparoz
Contributor
oparoz commented Mar 29, 2016

Steps to reproduce

  1. Look at the hook for file_put_contents
  2. Now look at the hooks for fopen and file_get_contents

Expected behaviour

All targets should look like this:

'target' => $this->getMountPoint() . '/' . $path

Actual behaviour

2/3 of targets looks like this:

'target' => $this->getMountPoint() . $path

@rullzer @PVince81

@oparoz oparoz added this to the 9.0.1-current-maintenance milestone Mar 29, 2016
@oparoz oparoz changed the title from 2/3 of the hooks of OC\Files\Storage\Shared point to unexisting files to 2/3 of the hooks of OC\Files\Storage\Shared can point to unexisting files Mar 29, 2016
@PVince81
Collaborator

@nickvergessen any insights ? Does it affect the activity app in some way ?

@nickvergessen
Contributor

Activity doesn't listen to them.

@cmonteroluque cmonteroluque modified the milestone: 9.1-current, 9.0.1 Apr 8, 2016
@PVince81
Collaborator
PVince81 commented Jun 1, 2016

See comments in the PR #23648 (comment).

The issue with the missing path slash only exists in stable9, stable8.2. Please send PRs for these branches, thanks !

This qualifies as a backport @cmonteroluque @DeepDiver1975

@PVince81
Collaborator
PVince81 commented Jun 1, 2016

@oparoz Please send PRs for these branches, thanks !

(forgot to mention you 😄)

@oparoz
Contributor
oparoz commented Jun 1, 2016

@PVince81 - I'll try :) I hope it's just a matter of cherry-picking what I did.

@rullzer
Contributor
rullzer commented Jun 13, 2016

I think this is fixed now...

@oparoz
Contributor
oparoz commented Jun 13, 2016

@rullzer Did you patch stable9?

@oparoz
Contributor
oparoz commented Jun 13, 2016

I would only argue that on 9.1 there is a consistency issue. It doesn't make sense to build some paths with a slash while others don't have it. I'm assuming that because it's now working, the extra slash can be removed.

@rullzer
Contributor
rullzer commented Jun 14, 2016

No I did not patch stable9... are we just missing a / somewhere there?

@oparoz
Contributor
oparoz commented Jun 14, 2016

are we just missing a / somewhere there?

Yes, on 9, #23648 is needed because things break without it

But I would still fix the inconsistency on master and remove all slashes.

I'm happy to make the changes

@rullzer
Contributor
rullzer commented Jun 15, 2016

@oparoz please do

@PVince81 PVince81 added a commit that referenced this issue Jul 18, 2016
@oparoz @PVince81 oparoz + PVince81 Fix paths returned by \OC\Files\Storage\Shared hooks
Fixes #23620
9c83901
@PVince81 PVince81 added a commit that referenced this issue Jul 18, 2016
@oparoz @PVince81 oparoz + PVince81 Fix paths returned by \OC\Files\Storage\Shared hooks
Fixes #23620
f5efc29
@PVince81 PVince81 modified the milestone: 9.0.5, 9.0.4 Jul 18, 2016
@PVince81
Collaborator

stable9: #25519
stable8.2: #25520

Please review

@oparoz
Contributor
oparoz commented Jul 21, 2016

Thanks very much for that, it totally skipped my mind :S

@PVince81 PVince81 added a commit that referenced this issue Aug 11, 2016
@oparoz @PVince81 oparoz + PVince81 Fix paths returned by \OC\Files\Storage\Shared hooks
Fixes #23620
01dd38b
@PVince81 PVince81 added a commit that referenced this issue Aug 11, 2016
@oparoz @PVince81 oparoz + PVince81 Fix paths returned by \OC\Files\Storage\Shared hooks
Fixes #23620
da34398
@PVince81
Collaborator

All merged, closing

@PVince81 PVince81 closed this Sep 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment