-
Notifications
You must be signed in to change notification settings - Fork 184
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
Upload-only shares must not overwrite but create a separate file #1267
Comments
Ohh, this is a regression that only exists in cs3org/reva#822. I'll submit the fix there... |
now I'm confused... some other tests are failing now, the test is obviously overwriting the file but is expecting a 201 status and not 204. either I remembered the specs wrong and so does whoever wrote the put.go code, or maybe the old API was already returning the wrong value for public links... needs further research... I've enabled the tests that work correctly now on owncloud/core#37552 The remaining ones are here:
|
Here is another test for uploading + overwriting a file in a another non-public link context, and here we expect 204: https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperations/uploadToShare.feature#L111 I suspect that the public webdav API when it was added was not consistent with the non-public one, but the test got added to comply with the buggy result. For the time being we could write a separate set of tests for OCIS that requires 204 and later on we might need to fix OC 10 public webdav to also return 204 on overwrite. |
ownCloud 10tldr; all is fine there: creation returns 201 on all endpoints and overwrite returns 204. Logged-in userFile creation
File overwrite
Public folder share old public Webdav APIFile creation
File overwrite
Public folder share new public Webdav APIFile creation
File overwrite
|
OCISUsing cs3org/reva#877 Also seems to work fine File creation
File overwrite
|
I rechecked owncloud/core#37552 and those tests still fail:
they are related to upload-only permissions. |
Upload-only is more complicated than just the status code. The bug is that the current reva code is overwriting an existing file. The way it's done in OC 10 is that the server is detecting conflicts and then automatically renames the file in the backend, and since it's creating a new file the status becomes 201. So this is more about security. |
This is already covered by the tests with tag |
note: need to have a look at the Webdav code from OC 10. As far as I remember there's an extra header "OC-Autorename" to trigger or notify that behavior. |
this feels less important than https://github.com/owncloud/ocis-reva/issues/292 and as the latter is not planned until later, let's defer this one as well |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions. |
Discussion in REVA cs3org/reva#2480 |
fixed via #8340 |
Discovered while running tests/acceptance/features/apiSharePublicLink1/changingPublicLinkShare.feature:171
The text was updated successfully, but these errors were encountered: