The php ISO8601 date format is not correct for XMLRPC dates #2885
Conversation
| global $conf; | ||
| global $USERINFO; | ||
|
|
||
| parent::setUp(); |
There was a problem hiding this comment.
You're calling this twice. Here and in line 15.
There was a problem hiding this comment.
Removed second instance.
inc/IXR_Library.php
Outdated
| @@ -822,6 +826,8 @@ function getXml() { | |||
| */ | |||
| class IXR_Date { | |||
|
|
|||
| const XMLRPC_ISO8601 = 'Ymd\TH:i:s'; | |||
There was a problem hiding this comment.
This omits the timezone completely. That seems wrong to me.
According to https://www.php.net/manual/en/class.datetimeinterface.php#datetime.constants.iso8601 we could use DateTime:ATOM instead.
There was a problem hiding this comment.
This is correct. See http://xmlrpc.scripting.com/spec.html. The timezone that is used should be specified in the server documentation.
There was a problem hiding this comment.
Hmm. Its weird. The spec example gives it without any time zone info, but at the same time calls it ISO8601. According to https://en.wikipedia.org/wiki/ISO_8601#Time_zone_designators there are different ways to represent the timezone, but it seems not to mention that it is optional.
Since the original problem in #2883 was your Java parser failing, could you check if it would work with DateTime:ATOM format? I would prefer having the timezone info intact.
There was a problem hiding this comment.
Ok, some strange stuff here. Seems that this issue was actually already fixed in master (as compared to the Greebo version.
The ISO8601 format works with the Java parser it's the missing XML decoration in the Greebo version which is the issue. Sorry for the noise.
inc/IXR_Library.php
Outdated
| @@ -406,7 +406,7 @@ function __construct($callbacks = false, $data = false, $wait = false) { | |||
| /** | |||
| * @param bool|string $data | |||
| */ | |||
| function serve($data = false) { | |||
| function serve($data = false, $test = false) { | |||
There was a problem hiding this comment.
Since we're using our own descendant of IXR_Server this could and should be implemented in our version dokuwiki\Remote\XmlRpcServer - or you could create your own child just for the test.
There was a problem hiding this comment.
The IXR_Server implementation prints to stdout so I don't know how to wrap this in the child and capture the result.
There was a problem hiding this comment.
Overwrite the output function and use output buffering is probably the easiest way.
There was a problem hiding this comment.
Done in commits below using a specific subclass for the test.
|
I should probably create a new PR to only add the test class for XmlRpcServer to remove the commit noise. |
|
I think it's OK if you have this many commits. If you don't like too many commits, you can rearrange the branch and do a force-push, or reopen a PR... Whichever works for you are fine. |
|
@phy25 I am also fine with this as it shows the evolution of the PR, but not everyone agrees. Some projects value a "clean" timeline. |
|
My testing really needs some work: just tested again with master on my personal server and with current code state I get on my Java client: |
|
Same error with |
|
With |
|
I will create a new PR with the XmlRpcServer test class and the ISO date without |
Fix the issue that incorrect date format is used in XMLRPC responses (issue #2883).