Allow DAV Object properties #30368
Merged
Allow DAV Object properties #30368
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tcitworld
added
bug
3. to review
tcitworld
requested review from
ChristophWurst,
icewind1991,
nickvergessen and
skjnldsv
tcitworld
commented
Dec 21, 2021
| public function run(IOutput $output) { | ||
| $query = $this->connection->getQueryBuilder(); | ||
| $updated = $query->delete('properties') | ||
| ->where($query->expr()->eq('propertyname', $query->createNamedParameter(self::RESOURCE_TYPE_PROPERTY))) |
Member
Author
There was a problem hiding this comment.
Important note : I chose to delete only {DAV:}resourcetype properties because they were the ones involved in the calendar issue, but there might be more concerned propertynames.
I didn't want to directly remove the condition on propertyname in case there's some legitimate Object propertyvalues, (which seems however unlikely).
Member
Author
There was a problem hiding this comment.
Found two extra property names affected:
{http://calendarserver.org/ns/}me-card{urn:ietf:params:xml:ns:caldav}schedule-calendar-transp
tcitworld
force-pushed
the
dav-allow-object-properties
branch
from
aa2bfe9 to
56bafc3
Compare
tcitworld
force-pushed
the
dav-allow-object-properties
branch
from
56bafc3 to
82a14e6
Compare
Member
Author
|
I think this is a better way to handle things than converting to string #30771 |
tcitworld
requested review from
a team and
blizzz
and removed request for
a team
Merged
Merged
tcitworld
force-pushed
the
dav-allow-object-properties
branch
from
82a14e6 to
175fb56
Compare
come-nc
reviewed
May 16, 2022
come-nc
requested changes
May 16, 2022
The current implementation only saves them as string. It seems they can be more complex than that, and that objects were saved directly. You may find such objects saved in some production databases by executing: ```sql SELECT * from oc_properties where propertyvalue = 'Object'; ``` This commit adds a repair job to clean all of these "broken" properties values, adds a new database column to save the type of the property, and handles converting from and to correct values. Implementation is very similar to SabreDAV's own PDO backend: https://github.com/nextcloud/3rdparty/blob/4921806dfb1c5c309eac60195ed34e2749baf3c1/sabre/dav/lib/DAV/PropertyStorage/Backend/PDO.php Signed-off-by: Thomas Citharel <[email protected]>
tcitworld
force-pushed
the
dav-allow-object-properties
branch
from
175fb56 to
bd8b213
Compare
come-nc
approved these changes
May 16, 2022
PVince81
approved these changes
Jun 10, 2022
Member
PVince81
left a comment
PVince81
left a comment
There was a problem hiding this comment.
Makes sense, old known issue 👍
Also will likely solve issues with Oracle when storing complex props as I rembember issues there
Member
|
25 only as it adds a new column and might be an expensive upgrade |
tcitworld
added a commit
that referenced
this pull request
Jun 30, 2023
The propertyvalue column can contain null 0x00 characters values because of serializing PHP objects since #30368. This truncates data in text fields, but not blob fields. We start by removing invalid value and altering the column to match the new type. That's what Sabre PDO's being doing in the first place 🙈 Closes #37754 Signed-off-by: Thomas Citharel <[email protected]>
tcitworld
added a commit
that referenced
this pull request
Jul 2, 2023
The propertyvalue column can contain null 0x00 characters values because of serializing PHP objects since #30368. This truncates data in text fields, but not blob fields. We start by removing invalid value and altering the column to match the new type. That's what Sabre PDO's being doing in the first place 🙈 Closes #37754 Signed-off-by: Thomas Citharel <[email protected]>
tcitworld
added a commit
that referenced
this pull request
Jul 2, 2023
The propertyvalue column can contain null 0x00 characters values because of serializing PHP objects since #30368. This truncates data in text fields, but not blob fields. We start by removing invalid value and altering the column to match the new type. That's what Sabre PDO's being doing in the first place 🙈 Closes #37754 Signed-off-by: Thomas Citharel <[email protected]>
8 tasks
6 tasks
tcitworld
added a commit
that referenced
this pull request
Oct 15, 2024
The propertyvalue column can contain null 0x00 characters values because of serializing PHP objects since #30368. This truncates data in text fields, but not blob fields. We start by removing invalid value and altering the column to match the new type. That's what Sabre PDO's being doing in the first place 🙈 Closes #37754 Signed-off-by: Thomas Citharel <[email protected]>
tcitworld
added a commit
that referenced
this pull request
Oct 15, 2024
The propertyvalue column can contain null 0x00 characters values because of serializing PHP objects since #30368. This truncates data in text fields, but not blob fields. We start by removing invalid value and altering the column to match the new type. That's what Sabre PDO's being doing in the first place 🙈 Closes #37754 Signed-off-by: Thomas Citharel <[email protected]>
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current implementation only saves them as string. It seems they can be more complex than that, and that objects were saved directly.
You may find such objects saved in some production databases by executing:
This seems to cause issues in some particular cases (nextcloud/calendar#3551) like the calendar trashbin because of things like this and (apparently ?) Apple devices:
server/apps/dav/lib/CalDAV/CalDavBackend.php
Lines 3208 to 3211 in 0b01103
This commit adds a repair job to clean all of these "broken" properties values, adds a new database column to save the type of the property, and handles converting from and to correct values.
Implementation is very similar to SabreDAV's own PDO backend: https://github.com/nextcloud/3rdparty/blob/4921806dfb1c5c309eac60195ed34e2749baf3c1/sabre/dav/lib/DAV/PropertyStorage/Backend/PDO.php
Fixes nextcloud/calendar#3858 (comment)