Skip to content

Conversation

@bastien-roucaries
Copy link
Contributor

Hi

To be tested after a merge on your vobject tree

@guimard

Documentation does not work with trixie docker. Fix it.

Add also some place to add local volume

Signed-off-by: Bastien Roucariès <[email protected]>
Will help testing

Signed-off-by: Bastien Roucariès <[email protected]>
Signed-off-by: Bastien Roucariès <[email protected]>
Signed-off-by: Bastien Roucariès <[email protected]>
Signed-off-by: Bastien Roucariès <[email protected]>
Getting full stack in dev mode is a must to do

Signed-off-by: Bastien Roucariès <[email protected]>
According to documentation sender should not be null

It will likely crash newer version of sabre

Signed-off-by: Bastien Roucariès <[email protected]>
Signed-off-by: Bastien Roucariès <[email protected]>
- Switched to using WildcardEmitter (Sabre/Dav #896)
- need _SERVER to be populated for test (sabre/http)

Signed-off-by: Bastien Roucariès <[email protected]>
Signed-off-by: Bastien Roucariès <[email protected]>
@chibenwa
Copy link
Member

Hello

I'm back from vacations and am delighted with this work. Thanks a lot for putting it together timely!

As explained in linagora/sabre-vobject#2 (comment) I encounter difficulties to access the https://github.com/bastien-roucaries/sabre-vobject repo => 404

While I have no issues with https://github.com/bastien-roucaries/esn-sabre

Is the visibility / access of the project set correctly?

As is it prevents me from pulling the code and test it locally...

Thanks!

@bastien-roucaries
Copy link
Contributor Author

bastien-roucaries commented Aug 19, 2025 via email

@chibenwa
Copy link
Member

chibenwa commented Aug 19, 2025

Hello @bastien-roucaries

I did succeed to run integration tests atop this changeset using waiting-merges-4.2.2 branch of your vobject repository

It goes overall OK the IT tests (to be published : https://github.com/linagora/twake-calendar-integration-tests )

image

I uncovered the following issues (clear regressions):

  • User can no longer create events with external users. CalDavTest::inboxShouldContainInvites did fail creating an event with the following property:
 ATTENDEE;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;ROLE=REQ-PARTICIPANT;CUTYPE=INDIVI
  DUAL;CN=Benoît TELLIER:mailto:[email protected]

And did yield the following log:

 sabre_dav 2025/08/19 17:04:10 [error] 14#14: *420 FastCGI sent in stderr: "PHP message: 3.7;Could not find principal for mailto:[email protected].
sabre_dav PHP message: 3.7;Could not find principal for mailto:[email protected]" while reading response header from upstream, client: 192.168.144.1, server: _, request: "PUT /calendars/68a4ae8a37fd7609fc1855bc/68a4ae8a37fd7609fc1855bc/abcd.ics HTTP/1.1", upstream: "fastcgi://unix:/var/run/php/php7.0-fpm.sock:", host: "192.168.144.6"

Looks like we can no longer invite external users...

  • On big issues HEAD on CardDAV did fail (CardDavTest::headShouldReturnFound) and returns a 500
sabre_dav [19-Aug-2025 16:57:43] WARNING: [pool www] child 24 said into stderr: "[2025-08-19 16:57:43] EsnSabre.CRITICAL: Uncaught exception {"exception":"[object] (Sabre\\VObject\\EofException(code: 0): End of document reached prematurely at /var/www/vendor/sabre/vobject/lib/Parser/MimeDir.php:284)"} []"

There are some minor issues that causes the tests to fail:

  • dtsamp is always rewitten (value mismatch & position) => test adaptations are needed
  • DAV: propfind always include an extra property: <d:propstat><d:prop><d:getlastmodified/><d:getcontentlength/><d:quota-used-bytes/><d:quota-available-bytes/><d:getetag/><d:getcontenttype/></d:prop><d:status>HTTP/1.1 404 Not Found</d:status></d:propstat>. (naive question) Is it normal ?
  • Somehow we get mismatches on getetag are no longer ignored... => test adaptations are needed
  • sabre + vobject version changes => test adaptations are needed

FYI I created a task for some linagora employees to work on adapting the tests in order to lift these edge cases CF linagora/twake-calendar-integration-tests#1

@bastien-roucaries
Copy link
Contributor Author

 User can no longer create events with external users. CalDavTest::inboxShouldContainInvites did fail creating an event with the following property:

ATTENDEE;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;ROLE=REQ-PARTICIPANT;CUTYPE=INDIVI
DUAL;CN=Benoît TELLIER:mailto:[email protected]

And did yield the following log:

sabre_dav 2025/08/19 17:04:10 [error] 14#14: *420 FastCGI sent in stderr: "PHP message: 3.7;Could not find principal for mailto:[email protected].
sabre_dav PHP message: 3.7;Could not find principal for mailto:[email protected]" while reading response header from upstream, client: 192.168.144.1, server: _, request: "PUT /calendars/68a4ae8a37fd7609fc1855bc/68a4ae8a37fd7609fc1855bc/abcd.ics HTTP/1.1", upstream: "fastcgi://unix:/var/run/php/php7.0-fpm.sock:", host: "192.168.144.6"

Looks like we can no longer invite external users...

quickly check point to :
https://central.owncloud.org/t/dav-backend-error-when-providing-vcalendar-objects-inviting-seemingly-random-users-registered-on-the-owncloud-instance/31479/8

@bastien-roucaries
Copy link
Contributor Author

sabre_dav [19-Aug-2025 16:57:43] WARNING: [pool www] child 24 said into stderr: "[2025-08-19 16:57:43] EsnSabre.CRITICAL: Uncaught exception {"exception":"[object] (Sabre\VObject\EofException(code: 0): End of document reached prematurely at /var/www/vendor/sabre/vobject/lib/Parser/MimeDir.php:284)"} []"

Could you check that in this case TZ is empty instead of being null ? BTW a full stack will help

@bastien-roucaries
Copy link
Contributor Author

 User can no longer create events with external users. CalDavTest::inboxShouldContainInvites did fail creating an event with the following property:

ATTENDEE;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;ROLE=REQ-PARTICIPANT;CUTYPE=INDIVI
DUAL;CN=Benoît TELLIER:mailto:[email protected]
And did yield the following log:
sabre_dav 2025/08/19 17:04:10 [error] 14#14: *420 FastCGI sent in stderr: "PHP message: 3.7;Could not find principal for mailto:[email protected].
sabre_dav PHP message: 3.7;Could not find principal for mailto:[email protected]" while reading response header from upstream, client: 192.168.144.1, server: _, request: "PUT /calendars/68a4ae8a37fd7609fc1855bc/68a4ae8a37fd7609fc1855bc/abcd.ics HTTP/1.1", upstream: "fastcgi://unix:/var/run/php/php7.0-fpm.sock:", host: "192.168.144.6"
Looks like we can no longer invite external users...

quickly check point to : https://central.owncloud.org/t/dav-backend-error-when-providing-vcalendar-objects-inviting-seemingly-random-users-registered-on-the-owncloud-instance/31479/8

For this one is likely a problem on non escaping somewhere the space in Benoit Telier somewhere. I full task and getting special char seens will likely help

@bastien-roucaries
Copy link
Contributor Author

bastien-roucaries commented Aug 19, 2025

DAV: propfind always include an extra property: <d:propstat><d:prop><d:getlastmodified/><d:getcontentlength/><d:quota-used-bytes/><d:quota-available-bytes/><d:getetag/><d:getcontenttype/></d:prop><d:status>HTTP/1.1 404 Not Found</d:status></d:propstat>. (naive question) Is it normal ?

I have no the full propfind output but according to RFC (note the MUST):

Each propstat XML element MUST contain a prop element that contains one or more properties, and a status element that contains a single HTTP status line. The propstat element MUST be used inside a response element to indicate the status of properties for a resource.”

@chibenwa chibenwa mentioned this pull request Aug 29, 2025
@chibenwa chibenwa requested review from chibenwa and removed request for chibenwa August 29, 2025 06:25
@chibenwa chibenwa removed their assignment Aug 29, 2025
@guimard guimard requested a review from Copilot October 10, 2025 09:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ports the project from Sabre DAV 3.x to Sabre DAV 4.x, updating dependencies and adapting code to work with the newer framework version.

  • Updates Sabre DAV from version 3.2.2 to 4.0.3 and vobject dependency
  • Adapts event listener registration patterns to use new Sabre 4 syntax
  • Updates test configurations and adds proper error handling with try/finally blocks

Reviewed Changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
composer.json Updates Sabre DAV and vobject dependencies to version 4.x
lib/JSON/Plugin.php Changes event listener registration from 'beforeMethod' to 'beforeMethod:*'
lib/JSON/BasePlugin.php Updates event listener registration pattern for Sabre 4 compatibility
lib/DAV/XHttpMethodOverridePlugin.php Adapts event listener registration syntax
esn.php Updates event listener and changes exception method call
lib/Utils/Utils.php Adds proper try/finally block for error handling
tests/ Adds required sender/recipient properties and HTTP request parameters for test compatibility
scripts/generate_config.sh Improves shell script quoting
config.tests.json Updates configuration for Docker environment
compose-minimal.yml Adds Docker Compose configuration for minimal testing setup
README.md Updates documentation with new Docker setup instructions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

$server->on('beforeMethod', function() use ($e) {
throw new Sabre\DAV\Exception\ServiceUnavailable($e->getMessage());
$server->on('beforeMethod:*', function() use ($e) {
throw new Sabre\DAV\Exception\ServiceUnavailable($e->getTraceAsString());
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed from getMessage() to getTraceAsString() which will expose full stack traces in error responses. This could leak sensitive information and should use getMessage() instead.

Suggested change
throw new Sabre\DAV\Exception\ServiceUnavailable($e->getTraceAsString());
throw new Sabre\DAV\Exception\ServiceUnavailable($e->getMessage());

Copilot uses AI. Check for mistakes.
@chibenwa
Copy link
Member

Typos handled here

#92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants