-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fixes sharing to group ids with characters that are being url encoded #24515
fixes sharing to group ids with characters that are being url encoded #24515
Conversation
@@ -107,6 +107,7 @@ private function shareWith($shareable, $element) { | |||
return; | |||
} | |||
|
|||
$principal[2] = urldecode($principal[2]); |
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 breaks groups with a plus as it replaces them with
even if it might not be needed?
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.
Did you try this? Groups containing plusses should suffer the exact same issue.
But I think it reveals a different problem here … with and without this change groups containing plusses are not being offerend in the calendar share dialogue.
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.
Ok, i see the issue now.
It only works # because it does not decode into something else. But, as said above, this is a different problem, only unveiled here.
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.
@nickvergessen I added another commit fixing it. But as said, also without the first commit and the calendar changes, a search for groups containing a plus would end up with
<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
<s:exception>Sabre\DAV\Exception\NotFound</s:exception>
<s:message>Principal with name POW+WOW not found</s:message>
</d:error>
(I have not found a similar piece of code for other types like users or circles, i may know to little about the related dav bits)
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
fe59be1
to
622d028
Compare
I was finally having another look into this and had to do two more changes (also for carddav) essentially reverting #5298 which ironically was a fix for the same problem. I suppose it broke somewhere else. For me it works now with both contacts as well as addressbooks, and I tested with several groups: I shared addressbook as well as calendars to these groups. (here you also see that contacts has presentation issues - needs a fix like calendar got) and all were received: In the database URIs are saved with encoded name (as they should): My changes do not! change the behaviour of how the URIs are stored. What changes is the group search (Sharing/Backend changes) and the return of URIs in an properly encoded manner (other changes). Therefore I do not expect any effects on old shares, even though I do not have deep knowledge of the changes in between in these components or these components in itself. |
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.
🐘
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.
Code looks good
/backport to stable20 |
When you try to share a calendar to (local) group "Route#44" it will fail, because internally the group name was not decoded.
What also was and still is not the case that on failed sharing a proper error was being reported. I.e. at the moment you do not see that it was failing, only with a reload (or a complaint by one of the members :)). That might be a follow up PR.