Skip to content
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

[Bug]: Caldav limit tag on initial sync causes incorrect Caldav error response. #48678

Open
4 of 8 tasks
JeroenBer opened this issue Oct 13, 2024 · 5 comments
Open
4 of 8 tasks
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 28-feedback 29-feedback 30-feedback bug feature: caldav Related to CalDAV internals regression

Comments

@JeroenBer
Copy link

⚠️ This issue respects the following points: ⚠️

Bug description

OneCalendar users are experiencing errors when syncing caldav calendars with Nextcloud. This used to be working.

As OneCalendar developer I tried to reproduce the problem and for the latest versions of 28,29 and 30 the Caldav response is no longer working correctly when
doing a initial sync.

Hereby an overview of which versions are working correct/incorrect:

  • 28.0.9 Correct
  • 28.0.10 Incorrect
  • 28.0.11 Incorrect
  • 29.0.6 Correct
  • 29.0.7 Incorrect
  • 29.0.8 Incorrect
  • 30.0.0 Incorrect

Steps to reproduce

Sync caldav calendar data:

Request data

<sync-collection xmlns="DAV:">
  <sync-token />
  <sync-level>1</sync-level>
  <limit>
    <nresults>100</nresults>
  </limit>
  <prop>
    <getetag />
  </prop>
</sync-collection>

Actual response data (507):

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
	<s:exception>Interne serverfout</s:exception>
	<s:message>
		De server was niet in staat je aanvraag te verwerken.		Stuur de hieronder afgebeelde technische details naar de serverbeheerder wanneer dit opnieuw gebeurt.		Meer details in de serverlogging,			</s:message>

	<s:technical-details>
		<s:remote-address>172.17.0.1</s:remote-address>
		<s:request-id>du8YMujFjQr6HIRmBFm1</s:request-id>

		</s:technical-details>
</d:error>

Expected behavior

According to https://tools.ietf.org/html/rfc6578#section-3.11 the caldav server does not need to support limit tag but in that case the following 507 response data is expected:

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>OCA\DAV\Exception\UnsupportedLimitOnInitialSyncException</s:exception>
  <s:message/>
  <d:number-of-matches-within-limits/>
</d:error>

Nextcloud Server version

28

Operating system

None

PHP engine version

None

Web server

None

Database engine version

None

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

No response

List of activated Apps

No response

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

Perhaps there has been a minor update/fix of SabreDav that caused this problem in the newer 28/29/30 versions ?

@JeroenBer JeroenBer added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Oct 13, 2024
@joshtrichards
Copy link
Member

joshtrichards commented Oct 13, 2024

<s:request-id>du8YMujFjQr6HIRmBFm1</s:request-id>

The nextcloud.log should have a stack trace or some logging associated with this request id since it's an internal server error (I believe). Happen to have it handy from one of the instances that isn't responding as expected?

Enabling debug mode should also populate technical-details with more info.

I haven't looked too closely, but so far the changes introduced via #47770 & #47805 align most closely with your findings.

@kesselb
Copy link
Contributor

kesselb commented Oct 14, 2024

Thanks for your report, and sorry for the troubles.

I agree with Josh that likely #47770 and #47805 changed how we handle the UnsupportedLimitOnInitialSyncException.

@JeroenBer
Copy link
Author

JeroenBer commented Oct 14, 2024

Hi, I reproduced and looked in the nextcloud.log. This was what came up:

{"reqId":"HXLFwXcaYbWcv9Fym23z","level":3,"time":"2024-10-14T18:45:34+00:00","remoteAddr":"172.17.0.1","user":"admin","app":"webdav","method":"REPORT","url":"/remote.php/dav/calendars/admin/personal/","message":"Exception thrown: OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException","userAgent":"curl/7.54","version":"30.0.0.14","exception":{"Exception":"OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException","Message":"","Code":0,"Trace":[{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/Sync/Plugin.php","line":121,"function":"getChanges","class":"OCA\\DAV\\CalDAV\\Calendar","type":"->","args":[null,"1",100]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/Sync/Plugin.php","line":62,"function":"syncCollection","class":"Sabre\\DAV\\Sync\\Plugin","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/var/www/html/3rdparty/sabre/event/lib/WildcardEmitterTrait.php","line":89,"function":"Sabre\\DAV\\Sync\\{closure}","class":"Sabre\\DAV\\Sync\\Plugin","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/CorePlugin.php","line":690,"function":"emit","class":"Sabre\\DAV\\Server","type":"->","args":["report",["*** sensitive parameters replaced ***","*** sensitive parameters replaced ***","*** sensitive parameters replaced ***"]]},{"file":"/var/www/html/3rdparty/sabre/event/lib/WildcardEmitterTrait.php","line":89,"function":"httpReport","class":"Sabre\\DAV\\CorePlugin","type":"->","args":[["Sabre\\HTTP\\Request"],["Sabre\\HTTP\\Response"]]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php","line":472,"function":"emit","class":"Sabre\\DAV\\Server","type":"->","args":["method:REPORT",[["Sabre\\HTTP\\Request"],["Sabre\\HTTP\\Response"]]]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php","line":253,"function":"invokeMethod","class":"Sabre\\DAV\\Server","type":"->","args":[["Sabre\\HTTP\\Request"],["Sabre\\HTTP\\Response"]]},{"file":"/var/www/html/apps/dav/lib/Server.php","line":370,"function":"start","class":"Sabre\\DAV\\Server","type":"->","args":[]},{"file":"/var/www/html/apps/dav/appinfo/v2/remote.php","line":19,"function":"exec","class":"OCA\\DAV\\Server","type":"->","args":[]},{"file":"/var/www/html/remote.php","line":146,"args":["/var/www/html/apps/dav/appinfo/v2/remote.php"],"function":"require_once"}],"File":"/var/www/html/apps/dav/lib/CalDAV/Calendar.php","Line":369,"message":"","exception":{},"CustomMessage":"Exception thrown: OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException"}}

It seems indeed the Caldav "UnsupportedLimitOnInitialSyncException".

Probably the changes in #47770 or #47805 changed this behavior and is now producing incorrect exception behavior for Caldav. In my opinion this is clearly a bug, and it might also potentially cause issues in other Caldav errors.

We are receiving more and more mails from people affected by this behavior. Is this something that can be fixed soon ?

@kesselb
Copy link
Contributor

kesselb commented Oct 15, 2024

cc @artonge do you have a tip for us how to prevent the default exception handler to overwrite our exception?

@artonge
Copy link
Contributor

artonge commented Oct 15, 2024

Hmm, maybe we should display the exception for a given set of HTTP code. Probably here:

public function generateBody(\Throwable $ex, int $httpCode): mixed {
if ($this->acceptHtml()) {
$templateName = 'exception';
$renderAs = 'guest';
if ($httpCode === 403 || $httpCode === 404) {
$templateName = (string)$httpCode;
}
} else {
$templateName = 'xml_exception';
$renderAs = null;
$this->server->httpResponse->setHeader('Content-Type', 'application/xml; charset=utf-8');
}
$debug = $this->config->getSystemValueBool('debug', false);
$content = new OC_Template('core', $templateName, $renderAs);
$content->assign('title', $this->server->httpResponse->getStatusText());
$content->assign('remoteAddr', $this->request->getRemoteAddress());
$content->assign('requestID', $this->request->getId());
$content->assign('debugMode', $debug);
$content->assign('errorClass', get_class($ex));
$content->assign('errorMsg', $ex->getMessage());
$content->assign('errorCode', $ex->getCode());
$content->assign('file', $ex->getFile());
$content->assign('line', $ex->getLine());
$content->assign('exception', $ex);
return $content->fetchPage();
}

Update that method to display or not the exception based on the HTTP code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 28-feedback 29-feedback 30-feedback bug feature: caldav Related to CalDAV internals regression
Projects
Status: 📄 To do
Development

No branches or pull requests

4 participants