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

Cannot store complex custom Webdav property values + wrong values + Oracle failure #32670

Closed
PVince81 opened this issue Sep 11, 2018 · 17 comments · Fixed by #37314
Closed

Cannot store complex custom Webdav property values + wrong values + Oracle failure #32670

PVince81 opened this issue Sep 11, 2018 · 17 comments · Fixed by #37314
Labels
Milestone

Comments

@PVince81
Copy link
Contributor

PVince81 commented Sep 11, 2018

Steps

  1. Create a file "proppatch.xml" with the contents <?xml version="1.0" encoding="utf-8" ?><propertyupdate xmlns='DAV:'><set><prop><t:valnspace xmlns:t='http://example.com/neon/litmus/'><foo xmlns='http://bar'/></t:valnspace></prop></set></propertyupdate> which sets a complex value
  2. Create a folder "test" in the OC instance
  3. curl -k -D - -u admin:admin -X PROPPATCH -H "Content-Type: text/xml" --data-binary "@proppatch.xml" "http://localhost/owncloud/remote.php/webdav/test"
  4. Create a file "propfind.xml" with the contents <?xml version="1.0"?><a:propfind xmlns:a="DAV:" xmlns:oc="http://owncloud.org/ns"><a:prop><t:valnspace xmlns:t='http://example.com/neon/litmus/'/> </a:prop></a:propfind>
  5. curl -k -D - -u admin:admin -X PROPFIND -H "Content-Type: text/xml" --data-binary "@propfind-oracle.xml" "http://localhost/owncloud/remote.php/webdav/test"
  6. Check the "oc_properties" database table

Expected result

oc_properties table must contain <foo xmlns='http://bar'/> or something alike.
the PROPFIND response should contain <foo xmlns='http://bar'/>.

Actual result

With MySQL

oc_properties table contain the string "Object" as value.
the PROPFIND response contains <x1:valnspace xmlns:x1="http://example.com/neon/litmus/">Object</x1:valnspace>

With Oracle

PROPPATCH fails:


{"reqId":"u35FUAVYSNddV4jLeZoS","level":4,"time":"2018-09-10T20:50:17+00:00","remoteAddr":"172.18.0.4","user":"admin","app":"webdav","method":"PROPPATCH","url":"\/remote.php\/webdav\/litmus\/prop2","message":"Exception: An exception occurred while executing 'INSERT INTO \"oc_properties\" (\"fileid\",\"propertyname\",\"propertyvalue\") VALUES(?,?,?)' with params [123, \"{http:\\\/\\\/example.com\\\/neon\\\/litmus\\\/}valnspace\", {}]:\n\nORA-01008: not all variables bound: {\"Exception\":\"Doctrine\\\\DBAL\\\\Exception\\\\DriverException\",\"Message\":\"An exception occurred while executing 'INSERT INTO \\\"oc_properties\\\" (\\\"fileid\\\",\\\"propertyname\\\",\\\"propertyvalue\\\") VALUES(?,?,?)' with params [123, \\\"{http:\\\\\\\/\\\\\\\/example.com\\\\\\\/neon\\\\\\\/litmus\\\\\\\/}valnspace\\\", {}]:\\n\\nORA-01008: not all variables bound\",\"Code\":0,\"Trace\":\" #0 \\\/drone\\\/src\\\/lib\\\/composer\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/DBALException.php(128): Doctrine\\\\DBAL\\\\Driver\\\\AbstractOracleDriver->convertException('An exception oc...', Object(Doctrine\\\\DBAL\\\\Driver\\\\OCI8\\\\OCI8Exception))\\n #1 \\\/drone\\\/src\\\/lib\\\/composer\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/Connection.php(1015): Doctrine\\\\DBAL\\\\DBALException::driverExceptionDuringQuery(Object(Doctrine\\\\DBAL\\\\Driver\\\\OCI8\\\\Driver), Object(Doctrine\\\\DBAL\\\\Driver\\\\OCI8\\\\OCI8Exception), 'INSERT INTO \\\"oc...', Array)\\n #2 \\\/drone\\\/src\\\/lib\\\/private\\\/DB\\\/Connection.php(207): Doctrine\\\\DBAL\\\\Connection->executeUpdate('INSERT INTO \\\"oc...', Array, Array)\\n #3 \\\/drone\\\/src\\\/apps\\\/dav\\\/lib\\\/DAV\\\/FileCustomPropertiesBackend.php(182): OC\\\\DB\\\\Connection->executeUpdate('INSERT INTO \\\"oc...', Array)\\n #4 \\\/drone\\\/src\\\/apps\\\/dav\\\/lib\\\/DAV\\\/AbstractCustomPropertiesBackend.php(206): OCA\\\\DAV\\\\DAV\\\\FileCustomPropertiesBackend->updateProperties('litmus\\\/prop2', Object(OCA\\\\DAV\\\\Connector\\\\Sabre\\\\File), Array)\\n #5 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/PropPatch.php(317): OCA\\\\DAV\\\\DAV\\\\AbstractCustomPropertiesBackend->OCA\\\\DAV\\\\DAV\\\\{closure}(Array)\\n #6 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/PropPatch.php(245): Sabre\\\\DAV\\\\PropPatch->doCallBackMultiProp(Array, Object(Closure))\\n #7 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1265): Sabre\\\\DAV\\\\PropPatch->commit()\\n #8 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/CorePlugin.php(385): Sabre\\\\DAV\\\\Server->updateProperties('litmus\\\/prop2', Array)\\n #9 [internal function]: Sabre\\\\DAV\\\\CorePlugin->httpPropPatch(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n #10 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/EventEmitterTrait.php(105): call_user_func_array(Array, Array)\\n #11 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(479): Sabre\\\\Event\\\\EventEmitter->emit('method:PROPPATC...', Array)\\n #12 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(254): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n #13 \\\/drone\\\/src\\\/apps\\\/dav\\\/appinfo\\\/v1\\\/webdav.php(64): Sabre\\\\DAV\\\\Server->exec()\\n #14 \\\/drone\\\/src\\\/remote.php(165): require_once('\\\/drone\\\/src\\\/apps...')\\n #15 {main}\",\"File\":\"\\\/drone\\\/src\\\/lib\\\/composer\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/Driver\\\/AbstractOracleDriver.php\",\"Line\":76}"}
--

This is likely because the complex value cannot be converted to string properly, so some process in Doctrine attempts to transform it to string. In MySQL it becomes Object, with Oracle it becomes an empty string or the weird {} value in the query.

Version

ownCloud 10.0.10 RC1

Discovered while debugging Oracle Litmus failure with PROPPATCH, see #32639 (comment)

Not sure if this ever worked before.
I'm really surprised Litmus lets this pass for MySQL.

I suspect this never worked before as we never really tested with complex properties.

@PVince81
Copy link
Contributor Author

We could attempt to convert the Sabre\DAV\Xml\Property\Complex instance to a better string representation.

One challenge is that when reading it back we need to know it was a complex property and not a plain string. Might need an extra flag in the oc_properties table to mark complex properties.

We could also look at how the Sabre default properties backend implementations handle this.

@PVince81
Copy link
Contributor Author

This blocks Litmus tests for Oracle #32639

@PVince81
Copy link
Contributor Author

It should be possible to write an acceptance test that does a PROPPATCH of a custom property to cover this.

@ownclouders
Copy link
Contributor

GitMate.io thinks the contributor most likely able to help you is @DeepDiver1975.

Possibly related issues are #11869 (webDAV failures), #12862 (Cannot get custom WebDAV properties back after setting them), #10247 (WebDAV permission attribute wrong), #6477 (Use WebDAV custom properties to tag/categorize files), and #256 (getetag property value in WebDAV response lacks quotes).

@PVince81
Copy link
Contributor Author

Acceptance test to trigger this here: #32675

@PVince81
Copy link
Contributor Author

might need a new column with a flag that tells us whether the value is complex or not ?

@pmaier1
Copy link
Contributor

pmaier1 commented Mar 11, 2019

Oracle DB support is a blocker for the public release, IMO. Putting this into "development" milestone therefore. Can you estimate the effort, please?

@pmaier1 pmaier1 modified the milestones: backlog, development Mar 11, 2019
@PVince81 PVince81 modified the milestones: development, backlog Mar 11, 2019
@PVince81
Copy link
Contributor Author

@VicDeo can you estimate ?

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 11, 2019

@pmaier1 note: so far there is no known app out there that actually use the feature in question. This is only about Webdav compliance as tested by litmus.

@micbar
Copy link
Contributor

micbar commented Mar 11, 2019

@PVince81 the issue that we had in files_lifecycle is a wrong escaping of our custom Properties in the query builder.

							->where(
								$qb2->expr()->in(
									'propertyname',
									[
										$qb2->expr()->literal(ArchivePlugin::UPLOAD_TIME),
										$qb2->expr()->literal(ArchivePlugin::RESTORED_TIME)
									]
								)
							)

@VicDeo
Copy link
Member

VicDeo commented Mar 11, 2019

We could also look at how the Sabre default properties backend implementations handle this.

I might be wrong but it is supposed that property should know how convert itself into something usable for DB:
https://github.com/sabre-io/dav/blob/master/lib/DAV/PropertyStorage/Backend/PDO.php#L124-L136

No idea for estimates. Let's say 2md

@VicDeo
Copy link
Member

VicDeo commented Mar 11, 2019

oops. I'm wrong (but not 100%).
There are three different value types (scalar, Complex and Object) and they are stored differently:
https://github.com/sabre-io/dav/blob/master/lib/DAV/PropertyStorage/Backend/PDO.php#L143-L151

type is stored next to the property itself and the PROPFIND value is returned according to the type:
https://github.com/sabre-io/dav/blob/master/lib/DAV/PropertyStorage/Backend/PDO.php#L94-L105

@phil-davis
Copy link
Contributor

phil-davis commented Dec 12, 2019

In PHP 7.4 when we run the scenario for this and with MySQL DB:

  @issue-32670
  Scenario Outline: Setting custom complex DAV property and reading it                                                                                             # /home/phil/git/owncloud/core/tests/acceptance/features/apiWebdavProperties/setFileProperties.feature:24
    Given using <dav_version> DAV path                                                                                                                             # FeatureContext::usingOldOrNewDavPath()
    And user "user0" has uploaded file "filesForUpload/textfile.txt" to "/testcustomprop.txt"                                                                      # FeatureContext::userHasUploadedAFileTo()
    And user "user0" has set property "very-custom-prop" with namespace "x1='http://whatever.org/ns'" of file "/testcustomprop.txt" to "<foo xmlns='http://bar'/>" # WebDavPropertiesContext::userHasSetPropertyWithNamespaceOfEntryTo()
    When user "user0" gets a custom property "very-custom-prop" with namespace "x1='http://whatever.org/ns'" of file "/testcustomprop.txt"                         # WebDavPropertiesContext::userGetsPropertiesOfFile()
    Then the response should contain a custom "very-custom-prop" property with namespace "x1='http://whatever.org/ns'" and value "Object"                          # WebDavPropertiesContext::theResponseShouldContainACustomPropertyWithValue()

    Examples:
      | dav_version |
      | old         |
        "very-custom-prop" has a value "" but "Object" expected
        Failed asserting that two strings are equal.
        --- Expected
        +++ Actual
        @@ @@
        -'Object'
        +''

--- Failed scenarios:

    /home/phil/git/owncloud/core/tests/acceptance/features/apiWebdavProperties/setFileProperties.feature:33

1 scenario (1 failed)
7 steps (6 passed, 1 failed)

and and the backend throws an exception:

Object of class Sabre\DAV\Xml\Property\Complex could not be converted to string: {"Exception":"Doctrine\\DBAL\\DBALException","Message":"An exception occurred while executing 'INSERT INTO `oc_properties` (`fileid`,`propertyname`,`propertyvalue`) VALUES(?,?,?)' with params [2147483793, \"{http:\\\/\\\/whatever.org\\\/ns}very-custom-prop\", {}]:\n\nObject of class Sabre\\DAV\\Xml\\Property\\Complex could not be converted to string","Code":0,"Trace":"#0 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/doctrine\/dbal\/lib\/Doctrine\/DBAL\/DBALException.php(145): Doctrine\\DBAL\\DBALException::wrapException()\n#1 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/doctrine\/dbal\/lib\/Doctrine\/DBAL\/Connection.php(1065): Doctrine\\DBAL\\DBALException::driverExceptionDuringQuery()\n#2 \/home\/phil\/git\/owncloud\/core\/lib\/private\/DB\/Connection.php(207): Doctrine\\DBAL\\Connection->executeUpdate()\n#3 \/home\/phil\/git\/owncloud\/core\/apps\/dav\/lib\/DAV\/FileCustomPropertiesBackend.php(193): OC\\DB\\Connection->executeUpdate()\n#4 \/home\/phil\/git\/owncloud\/core\/apps\/dav\/lib\/DAV\/AbstractCustomPropertiesBackend.php(214): OCA\\DAV\\DAV\\FileCustomPropertiesBackend->updateProperties()\n#5 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/sabre\/dav\/lib\/DAV\/PropPatch.php(295): OCA\\DAV\\DAV\\AbstractCustomPropertiesBackend->OCA\\DAV\\DAV\\{closure}()\n#6 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/sabre\/dav\/lib\/DAV\/PropPatch.php(230): Sabre\\DAV\\PropPatch->doCallBackMultiProp()\n#7 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/sabre\/dav\/lib\/DAV\/Server.php(1261): Sabre\\DAV\\PropPatch->commit()\n#8 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/sabre\/dav\/lib\/DAV\/CorePlugin.php(393): Sabre\\DAV\\Server->updateProperties()\n#9 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/sabre\/event\/lib\/WildcardEmitterTrait.php(96): Sabre\\DAV\\CorePlugin->httpPropPatch()\n#10 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/sabre\/dav\/lib\/DAV\/Server.php(464): Sabre\\DAV\\Server->emit()\n#11 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/sabre\/dav\/lib\/DAV\/Server.php(241): Sabre\\DAV\\Server->invokeMethod()\n#12 \/home\/phil\/git\/owncloud\/core\/lib\/composer\/sabre\/dav\/lib\/DAV\/Server.php(309): Sabre\\DAV\\Server->start()\n#13 \/home\/phil\/git\/owncloud\/core\/apps\/dav\/appinfo\/v1\/webdav.php(65): Sabre\\DAV\\Server->exec()\n#14 \/home\/phil\/git\/owncloud\/core\/remote.php(165): require_once('\/home\/phil\/git\/...')\n#15 {main}","File":"\/home\/phil\/git\/owncloud\/core\/lib\/composer\/doctrine\/dbal\/lib\/Doctrine\/DBAL\/DBALException.php","Line":172}

Maybe this is related to https://wiki.php.net/rfc/tostring_exceptions
PHP 7.4 seems to have some "enhanced" behaviour for __toString - probably in the past __toString on an object returned the literal string Object but now it throws an exception.

The scenario was passing on PHP 7.* before 7.4

@phil-davis
Copy link
Contributor

And litmus fails in PHP 7.4
https://drone.owncloud.com/owncloud/core/21962/8/14

21. propvalnspace......... 
21. propvalnspace......... FAIL (PROPPATCH of property with value defining namespace)

owncloud.log contains:

{"reqId":"vVrr8E2oIWzFzQGvXUha","level":4,"time":"2019-12-12T13:08:19+00:00","remoteAddr":"172.21.0.5","user":"admin","app":"webdav","method":"PROPPATCH","url":"\/remote.php\/webdav\/litmus\/prop2","message":"Exception: An exception occurred while executing 'INSERT INTO `oc_properties` (`fileid`,`propertyname`,`propertyvalue`) VALUES(?,?,?)' with params [123, \"{http:\\\/\\\/example.com\\\/neon\\\/litmus\\\/}valnspace\", {}]:\n\nObject of class Sabre\\DAV\\Xml\\Property\\Complex could not be converted to string: {\"Exception\":\"Doctrine\\\\DBAL\\\\DBALException\",\"Message\":\"An exception occurred while executing 'INSERT INTO `oc_properties` (`fileid`,`propertyname`,`propertyvalue`) VALUES(?,?,?)' with params [123, \\\"{http:\\\\\\\/\\\\\\\/example.com\\\\\\\/neon\\\\\\\/litmus\\\\\\\/}valnspace\\\", {}]:\\n\\nObject of class Sabre\\\\DAV\\\\Xml\\\\Property\\\\Complex could not be converted to string\",\"Code\":0,\"Trace\":\"#0 \\\/drone\\\/src\\\/lib\\\/composer\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/DBALException.php(145): Doctrine\\\\DBAL\\\\DBALException::wrapException()\\n#1 \\\/drone\\\/src\\\/lib\\\/composer\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/Connection.php(1065): Doctrine\\\\DBAL\\\\DBALException::driverExceptionDuringQuery()\\n#2 \\\/drone\\\/src\\\/lib\\\/private\\\/DB\\\/Connection.php(207): Doctrine\\\\DBAL\\\\Connection->executeUpdate()\\n#3 \\\/drone\\\/src\\\/apps\\\/dav\\\/lib\\\/DAV\\\/FileCustomPropertiesBackend.php(193): OC\\\\DB\\\\Connection->executeUpdate()\\n#4 \\\/drone\\\/src\\\/apps\\\/dav\\\/lib\\\/DAV\\\/AbstractCustomPropertiesBackend.php(214): OCA\\\\DAV\\\\DAV\\\\FileCustomPropertiesBackend->updateProperties()\\n#5 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/PropPatch.php(295): OCA\\\\DAV\\\\DAV\\\\AbstractCustomPropertiesBackend->OCA\\\\DAV\\\\DAV\\\\{closure}()\\n#6 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/PropPatch.php(230): Sabre\\\\DAV\\\\PropPatch->doCallBackMultiProp()\\n#7 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1261): Sabre\\\\DAV\\\\PropPatch->commit()\\n#8 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/CorePlugin.php(393): Sabre\\\\DAV\\\\Server->updateProperties()\\n#9 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/WildcardEmitterTrait.php(96): Sabre\\\\DAV\\\\CorePlugin->httpPropPatch()\\n#10 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(464): Sabre\\\\DAV\\\\Server->emit()\\n#11 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(241): Sabre\\\\DAV\\\\Server->invokeMethod()\\n#12 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(309): Sabre\\\\DAV\\\\Server->start()\\n#13 \\\/drone\\\/src\\\/apps\\\/dav\\\/appinfo\\\/v1\\\/webdav.php(65): Sabre\\\\DAV\\\\Server->exec()\\n#14 \\\/drone\\\/src\\\/remote.php(165): require_once('\\\/drone\\\/src\\\/apps...')\\n#15 {main}\",\"File\":\"\\\/drone\\\/src\\\/lib\\\/composer\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/DBALException.php\",\"Line\":172}"}

This is the same sort of object __toString problem.

@phil-davis
Copy link
Contributor

This stops litmus passing on MySQL/MariaDB with PHP 7.4

@VicDeo
Copy link
Member

VicDeo commented Apr 23, 2020

Oracle failure is addressed in #37298
This PR is enough to get successful 10.5 build.
But probably I need to invest more time in this issue next week to get it working.

@phil-davis
Copy link
Contributor

I added this to the ownCloud Server 10.5.0 project so that it gets noticed for a decision/action.

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 a pull request may close this issue.

7 participants