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 an XMLRPC uploading file issue #734

Merged
merged 3 commits into from
Feb 21, 2024
Merged

Conversation

crazytonyli
Copy link
Contributor

Description

Fixes wordpress-mobile/WordPress-iOS#22659

Testing Details

See the added unit tests.


  • Please check here if your pull request includes additional test coverage.
  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

@crazytonyli crazytonyli requested a review from mokagio February 21, 2024 21:22
@crazytonyli crazytonyli changed the title Xmlrpc double encoding issue Fix an XMLRPC uploading file issue Feb 21, 2024
Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

Approving to unblock. I checked it out locally and saw that the test fails with the original code.

However, I don't understand how this works 🤔 I've been looking into the WPXMLRPCEncoder method and a bit down the chain and I don't see anything that would result in the second call (however unnecessary) to it returning a different value from the first 🤔

@crazytonyli crazytonyli merged commit f335202 into trunk Feb 21, 2024
7 checks passed
@crazytonyli crazytonyli deleted the xmlrpc-double-encoding-issue branch February 21, 2024 22:48
@crazytonyli
Copy link
Contributor Author

The root cause is at the encodingInputStream function. The first call works as expected with full base64 encoded content. Since the first call reads the stream to its end, the second call start reading from the end, which of course returns nothing, and get an empty base64 encoded content.

@mokagio
Copy link
Contributor

mokagio commented Feb 22, 2024

Got it. Thanks for following up @crazytonyli

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URLSession feature flag leads to broken video uploads on self-hosted sites
2 participants