X Tutup
Skip to content

Fix decoding of calendars uri#37750

Merged
micbar merged 1 commit intomasterfrom
fix-decoding-calendar-uri
Aug 27, 2020
Merged

Fix decoding of calendars uri#37750
micbar merged 1 commit intomasterfrom
fix-decoding-calendar-uri

Conversation

@pako81
Copy link

@pako81 pako81 commented Jul 29, 2020 •
edited
Loading

Description

Fix decoding of calendars uri

Motivation and Context

This PR fixes decoding of calendars uri in case they do contain special characters, for instance, persönlich.

How Has This Been Tested?

Manually:

  • create a new user, log in as this user, go into the DB and update his oc_calendars.uri like: update oc_calendars set uri='persönlich' where id=$id. So basically by adding an umlaut in his calendar uri

  • as another user try to invite this user to an event --> without this fix, the event cannot be created and a 404 is shown in the stack trace. Reason for this is the uri not being properly decoded.

{"reqId":"2Dx37BA32uuaGHMNk4p8","level":0,"time":"2020-07-28T21:13:48+00:00","remoteAddr":"::1","user":"admin","app":"webdav","method":"PUT","url":"\/owncloud1050\/remote.php\/dav\/calendars\/admin\/personal\/ownCloud-QFB8XWSOMQ2AC7YKQTLV9.ics","message":"Exception: HTTP\/1.1 404 Node with name 'pers%c3%b6nlich' could not be found: {\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\NotFound\",\"Message\":\"Node with name 'pers%c3%b6nlich' could not be found\",\"Code\":0,\"Trace\":\"#0 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Tree.php(78): OCA\\\\DAV\\\\CalDAV\\\\CalendarHome->getChild('pers%c3%b6nlich')\\n#1 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/apps\\\/dav\\\/lib\\\/Tree.php(51): Sabre\\\\DAV\\\\Tree->getNodeForPath('calendars\\\/user2...')\\n#2 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/CalDAV\\\/Schedule\\\/Plugin.php(523): OCA\\\\DAV\\\\Tree->getNodeForPath('calendars\\\/user2...')\\n#3 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/WildcardEmitterTrait.php(89): Sabre\\\\CalDAV\\\\Schedule\\\\Plugin->scheduleLocalDelivery(Object(Sabre\\\\VObject\\\\ITip\\\\Message))\\n#4 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/CalDAV\\\/Schedule\\\/Plugin.php(350): Sabre\\\\DAV\\\\Server->emit('schedule', Array)\\n#5 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/CalDAV\\\/Schedule\\\/Plugin.php(626): Sabre\\\\CalDAV\\\\Schedule\\\\Plugin->deliver(Object(Sabre\\\\VObject\\\\ITip\\\\Message))\\n#6 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/CalDAV\\\/Schedule\\\/Plugin.php(337): Sabre\\\\CalDAV\\\\Schedule\\\\Plugin->processICalendarChange(NULL, Object(Sabre\\\\VObject\\\\Component\\\\VCalendar), Array, Array, true)\\n#7 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/WildcardEmitterTrait.php(89): Sabre\\\\CalDAV\\\\Schedule\\\\Plugin->calendarObjectChange(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response), Object(Sabre\\\\VObject\\\\Component\\\\VCalendar), 'calendars\\\/admin...', true, true)\\n#8 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/CalDAV\\\/Plugin.php(897): Sabre\\\\DAV\\\\Server->emit('calendarObjectC...', Array)\\n#9 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/CalDAV\\\/Plugin.php(766): Sabre\\\\CalDAV\\\\Plugin->validateICalendar('BEGIN:VCALENDAR...', 'calendars\\\/admin...', false, Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response), true)\\n#10 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/WildcardEmitterTrait.php(89): Sabre\\\\CalDAV\\\\Plugin->beforeCreateFile('calendars\\\/admin...', 'BEGIN:VCALENDAR...', Object(OCA\\\\DAV\\\\CalDAV\\\\Calendar), false)\\n#11 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1087): Sabre\\\\DAV\\\\Server->emit('beforeCreateFil...', Array)\\n#12 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/CorePlugin.php(504): Sabre\\\\DAV\\\\Server->createFile('calendars\\\/admin...', 'BEGIN:VCALENDAR...', NULL)\\n#13 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/WildcardEmitterTrait.php(89): Sabre\\\\DAV\\\\CorePlugin->httpPut(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#14 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(470): Sabre\\\\DAV\\\\Server->emit('method:PUT', Array)\\n#15 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(251): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#16 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/apps\\\/dav\\\/lib\\\/Server.php(329): Sabre\\\\DAV\\\\Server->start()\\n#17 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#18 \\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/remote.php(165): require_once('\\\/Users\\\/pasquale...')\\n#19 {main}\",\"File\":\"\\\/Users\\\/pasqualetripodi\\\/Sites\\\/owncloud1050\\\/apps\\\/dav\\\/lib\\\/CalDAV\\\/CalendarHome.php\",\"Line\":105}"}

With this fix, the event can be correctly created and 404 is not logged anymore.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised
  • Changelog item

@pako81 pako81 added this to the development milestone Jul 29, 2020
@pako81 pako81 self-assigned this Jul 29, 2020
@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #37750 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #37750      +/-   ##
============================================
- Coverage     64.71%   64.71%   -0.01%     
  Complexity    19386    19386              
============================================
  Files          1283     1283              
  Lines         75732    75736       +4     
  Branches       1333     1333              
============================================
+ Hits          49009    49011       +2     
- Misses        26331    26333       +2     
  Partials        392      392
Flag Coverage Δ Complexity Δ
#javascript 54.03% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.89% <0.00%> (-0.01%) 19386.00 <0.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/CalDAV/CalendarHome.php 0.00% <0.00%> (ø) 19.00 <0.00> (ø)
...tings/templates/panels/admin/persistentlocking.php 66.66% <0.00%> (-33.34%) 0.00% <0.00%> (ø%)
apps/files/lib/Capabilities.php 100.00% <0.00%> (ø) 2.00% <0.00%> (ø%)
settings/Panels/Admin/PersistentLocking.php 100.00% <0.00%> (ø) 4.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93ee30d...38bbfcf. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #37750 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #37750      +/-   ##
============================================
- Coverage     64.71%   64.71%   -0.01%     
  Complexity    19386    19386              
============================================
  Files          1283     1283              
  Lines         75732    75736       +4     
  Branches       1333     1333              
============================================
+ Hits          49009    49011       +2     
- Misses        26331    26333       +2     
  Partials        392      392
Flag Coverage Δ Complexity Δ
#javascript 54.03% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.89% <0.00%> (-0.01%) 19386.00 <0.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/CalDAV/CalendarHome.php 0.00% <0.00%> (ø) 19.00 <0.00> (ø)
...tings/templates/panels/admin/persistentlocking.php 66.66% <0.00%> (-33.34%) 0.00% <0.00%> (ø%)
apps/files/lib/Capabilities.php 100.00% <0.00%> (ø) 2.00% <0.00%> (ø%)
settings/Panels/Admin/PersistentLocking.php 100.00% <0.00%> (ø) 4.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93ee30d...38bbfcf. Read the comment docs.

@mmattel mmattel requested a review from phil-davis July 30, 2020 05:57
@mmattel
Copy link
Contributor

mmattel commented Jul 30, 2020

@phil-davis added you to the reviewers. I guess there are tests necessary to to make codecov happy...

@phil-davis
Copy link
Contributor

phil-davis commented Jul 30, 2020

My IDE tells me that there are no unit tests for class CalendarHome - someone would have to write unit tests "from scratch" for this class.

@mmattel
Copy link
Contributor

mmattel commented Aug 26, 2020

Any progress?

@phil-davis phil-davis requested a review from micbar August 26, 2020 13:41
@phil-davis phil-davis force-pushed the fix-decoding-calendar-uri branch from 38bbfcf to a0734e6 Compare August 26, 2020 13:42
@owncloud owncloud deleted a comment from update-docs bot Aug 26, 2020
@phil-davis phil-davis removed their request for review August 26, 2020 13:51
@phil-davis
Copy link
Contributor

phil-davis commented Aug 26, 2020

I rebased to get fresh CI. There are currently no unit tests for this stuff.

@micbar what to do?

@phil-davis phil-davis force-pushed the fix-decoding-calendar-uri branch from a0734e6 to 0f1c83b Compare August 26, 2020 13:54
@codecov
Copy link

codecov bot commented Aug 26, 2020

Codecov Report

Merging #37750 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #37750      +/-   ##
============================================
- Coverage     64.75%   64.75%   -0.01%     
  Complexity    19403    19403              
============================================
  Files          1285     1285              
  Lines         75816    75817       +1     
  Branches       1336     1336              
============================================
  Hits          49094    49094              
- Misses        26328    26329       +1     
  Partials        394      394
Flag Coverage Δ Complexity Δ
#javascript 54.06% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.93% <0.00%> (-0.01%) 19403.00 <0.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/CalDAV/CalendarHome.php 0.00% <0.00%> (ø) 19.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15b5ed6...0f1c83b. Read the comment docs.

@mmattel
Copy link
Contributor

mmattel commented Aug 27, 2020

codecov seems to be stuck - or?

@phil-davis
Copy link
Contributor

phil-davis commented Aug 27, 2020

codecov/patch is always stuck these days. And sometimes codecov/project also does not come. I have had long discussions in an issue with codecov. Last weekend I was trying stuff PRs #37832 #37833 but I also need to "get a life" and looking into that is "community contribution".

@mmattel
Copy link
Contributor

mmattel commented Aug 27, 2020

ci is ok, merging without codecov?

@micbar micbar merged commit b2592ef into master Aug 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-decoding-calendar-uri branch August 27, 2020 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@micbar micbar Awaiting requested review from micbar

Assignees

@pako81 pako81

Labels

3 - To Review

Projects

None yet

Milestone

development

Development

Successfully merging this pull request may close these issues.

4 participants

@pako81 @mmattel @phil-davis @micbar
X Tutup