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

fix xml ocs response for serializable objects #30417

Merged
merged 1 commit into from
Feb 23, 2024
Merged

Conversation

sualko
Copy link
Member

@sualko sualko commented Dec 27, 2021

If an OCS endpoint returns data which extends JsonSerializable the xml serialization should not fail.

Example in Nextcloud 23:

public function index(): DataResponse {
  return new DataResponse(
			"recommendations" => [/* object implementing JsonSerializable */]
		);
}

Will result in:

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>200</statuscode>
  <message>OK</message>
 </meta>
 <data>
  <recommendations/>
 </data>
</ocs>

The json response ?format=json is showing the list of recommendations as expected.

With the following error message in the logs:

XMLWriter::writeElement() expects parameter 2 to be string, object given at \/opt\/nextcloud\/23\/lib\/private\/AppFramework\/OCS\/BaseResponse.php#148

Discovered in nextcloud/recommendations#458

Edit: Of course this could also be done on the controller side, but I think it's so common and should be handled by the base response.

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

👍 🐘

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish technical debt and removed 3. to review Waiting for reviews labels Feb 23, 2024
@skjnldsv skjnldsv added this to the Nextcloud 29 milestone Feb 23, 2024
Signed-off-by: sualko <klaus@jsxc.org>
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv skjnldsv merged commit b0b4c7e into master Feb 23, 2024
157 checks passed
@skjnldsv skjnldsv deleted the sualko-patch-1 branch February 23, 2024 15:16
@blizzz blizzz mentioned this pull request Mar 5, 2024
@susnux
Copy link
Contributor

susnux commented Mar 18, 2024

/backport to stable28

@susnux
Copy link
Contributor

susnux commented Mar 18, 2024

Required for #44263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants