nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
27.34k stars 4.06k forks source link

Updating dav fails #2249

Closed MorrisJobke closed 7 years ago

MorrisJobke commented 7 years ago

cc @rullzer

Updating <dav> ...
Fix classification for calendar objects

 Starting ...
    0/0 [>---------------------------]   0%
 user1
    1/0 [->--------------------------]   0%
 user2
    2/0 [-->-------------------------]   0%
Sabre\VObject\InvalidDataException: Unsupported VALUE parameter for DTEND property. You supplied ""

https://github.com/nextcloud/3rdparty/blame/f2974c2e72b2ad5ab7ae745936c4d866405d2b61/sabre/vobject/lib/Document.php#L216

I will look up the entries in the database.

LukasReschke commented 7 years ago

@karlitschek FYI. Blocker for beta. Leaves a somewhat destroyed instance. (but of course everybody has backups 😉 )

MorrisJobke commented 7 years ago

Maybe we should also check for empty string (in the isset)

karlitschek commented 7 years ago

nrgh

MorrisJobke commented 7 years ago

I will look into this.

MorrisJobke commented 7 years ago
select * from oc_calendars where principaluri = 'principals/users/user2';
+----+------------------------+-------------------+-------------------+-----------+------------------+---------------+---------------+----------+--------------+-------------+
| id | principaluri           | displayname       | uri               | synctoken | description      | calendarorder | calendarcolor | timezone | components   | transparent |
+----+------------------------+-------------------+-------------------+-----------+------------------+---------------+---------------+----------+--------------+-------------+
| 19 | principals/users/user2 | Contact birthdays | contact_birthdays |         1 | NULL             |             0 | #FFFFCA       | NULL     | VEVENT,VTODO |           0 |
|  2 | principals/users/user2 | Default calendar  | defaultcalendar   |         1 | Default calendar |             0 | NULL          | NULL     | VEVENT,VTODO |           0 |
+----+------------------------+-------------------+-------------------+-----------+------------------+---------------+---------------+----------+--------------+-------------+
MorrisJobke commented 7 years ago

There are no entries in those calendars.

MorrisJobke commented 7 years ago

Stack trace:


#0 3rdparty/sabre/vobject/lib/Parser/MimeDir.php(453): Sabre\VObject\Document->createProperty('DTEND', NULL, Array)
#1 3rdparty/sabre/vobject/lib/Parser/MimeDir.php(234): Sabre\VObject\Parser\MimeDir->readProperty('DTEND;VALUE=:20...')
#2 3rdparty/sabre/vobject/lib/Parser/MimeDir.php(217): Sabre\VObject\Parser\MimeDir->parseLine('DTEND;VALUE=:20...')
#3 3rdparty/sabre/vobject/lib/Parser/MimeDir.php(181): Sabre\VObject\Parser\MimeDir->parseLine('DTEND;VALUE=:20...')
#4 3rdparty/sabre/vobject/lib/Parser/MimeDir.php(89): Sabre\VObject\Parser\MimeDir->parseDocument()
#5 3rdparty/sabre/vobject/lib/Reader.php(46): Sabre\VObject\Parser\MimeDir->parse('BEGIN:VCALENDAR...', 0)
#6 apps/dav/lib/CalDAV/CalDavBackend.php(1623): Sabre\VObject\Reader::read('BEGIN:VCALENDAR...')
#7 apps/dav/lib/Migration/Classification.php(72): OCA\DAV\CalDAV\CalDavBackend->getDenormalizedData('BEGIN:VCALENDAR...')
#8 apps/dav/lib/Migration/Classification.php(60): OCA\DAV\Migration\Classification->extractClassification('BEGIN:VCALENDAR...')
#9 apps/dav/lib/Migration/Classification.php(90): OCA\DAV\Migration\Classification->runForUser(Object(OC\User\User))
#10 lib/private/User/Manager.php(363): OCA\DAV\Migration\Classification->OCA\DAV\Migration\{closure}(Object(OC\User\User))
#11 apps/dav/lib/Migration/Classification.php(91): OC\User\Manager->callForAllUsers(Object(Closure))
#12 lib/private/Repair.php(92): OCA\DAV\Migration\Classification->run(Object(OC\Repair))
#13 lib/private/legacy/app.php(1190): OC\Repair->run()
#14 lib/private/legacy/app.php(1125): OC_App::executeRepairSteps('dav', Array)
#15 lib/private/Updater.php(358): OC_App::updateApp('dav')
#16 lib/private/Updater.php(244): OC\Updater->doAppUpgrade()
#17 lib/private/Updater.php(123): OC\Updater->doUpgrade('11.0.0.2', '9.1.1.5')
#18 core/Command/Upgrade.php(261): OC\Updater->upgrade()
#19 3rdparty/symfony/console/Command/Command.php(256): OC\Core\Command\Upgrade->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#20 3rdparty/symfony/console/Application.php(818): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#21 3rdparty/symfony/console/Application.php(186): Symfony\Component\Console\Application->doRunCommand(Object(OC\Core\Command\Upgrade), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#22 3rdparty/symfony/console/Application.php(117): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#23 lib/private/Console/Application.php(160): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#24 console.php(90): OC\Console\Application->run()
#25 occ(11): require_once('/usr/share/weba...')
#26 {main}
MorrisJobke commented 7 years ago

I tried to clean up the entry:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Apple Inc.//Mac OS X 10.11.2//EN
CALSCALE:GREGORIAN
BEGIN:VEVENT
TRANSP:OPAQUE
DTEND;VALUE=:20151223T223000Z
LAST-MODIFIED:20151214T091032Z
ORGANIZER;CN="User 1":mailto:user1@example.com
UID:1234567890@example.com
DTSTAMP:20151214T091032Z
STATUS:CONFIRMED
SEQUENCE:0
SUMMARY:Ein Geburtstag 
DTSTART:20151223T173000Z
X-APPLE-TRAVEL-ADVISORY-BEHAVIOR:AUTOMATIC
CREATED:20151214T091032Z
END:VEVENT
END:VCALENDAR
MorrisJobke commented 7 years ago

I updated the entry with a minimal example that fails. I created a unit test that also fails, but for inserting the entry in the database - see https://github.com/nextcloud/server/commit/38545c5

MorrisJobke commented 7 years ago

DTEND;VALUE=:20151223T223000Z

I guess this is simply a wrong value. Is it possible to repair such stuff? Or should we simply ignore such entries?

@evert This happened after the update of SabreDav from 2.1.2 to 3.2. Is the library now more strict?

MorrisJobke commented 7 years ago
    /**
     * @param IUser $user
     */
    public function runForUser($user) {
        $principal = 'principals/users/' . $user->getUID();
        $calendars = $this->calDavBackend->getCalendarsForUser($principal);
        foreach ($calendars as $calendar) {
            $objects = $this->calDavBackend->getCalendarObjects($calendar['id']);
            foreach ($objects as $object) {
                $calObject = $this->calDavBackend->getCalendarObject($calendar['id'], $object['uri']);
                try {
                    $classification = $this->extractClassification($calObject['calendardata']);
                } catch (\Sabre\VObject\InvalidDataException $e) {
                    if ($e->getMessage() !== 'Unsupported VALUE parameter for DTEND property. You supplied ""') {
                        throw $e;
                    }
                }
                $this->calDavBackend->setClassification($object['id'], $classification);
            }
        }
    }

@rullzer Maybe use something like the above ;)

rullzer commented 7 years ago

So oC removed them.. quick fix is to remove them as well https://github.com/owncloud/core/commit/610d8f53cb5d445f58cbcdb60310f5484a033f1a

nickvergessen commented 7 years ago

Well than we can't promise the update from 10 to 12 I guess? Or we simply need to make sure, that the upgrade step is only executed once?

rullzer commented 7 years ago

Well the upgrade was only executed from 9 to 10. So we would not be breaking anything yet.

MorrisJobke commented 7 years ago

Well than we can't promise the update from 10 to 12 I guess?

Do we promise this? I don't think so.

phsc84 commented 7 years ago

According to the admin manual, you do not.

But the feature to skip major releases when updating has been mentioned in a post about "planned update improvements" (see point 4) in the Nextcloud forum.