Problem/Motivation

I am working on a project that requires an external service which uses XML-RPC for requests. One API required date. It was not working. After debugging, I found that Drupal is not sending the date in xml request in correct format.

After digging I found that this xmlrpc_date_get_xml() is responsible for add a date in xml-rpc request. Also got some help from #321165-5: Exceptions for xmlrpc tests and date functions.

Looks like Drupal is missing an API which would format a date that can be used in xml-rpc request.

Proposed resolution

Write an API that would convert timestamp to a date format, that can be passed to xmlrpc_date_get_xml() which would add the date correctly in the xml-rpc request.

Remaining tasks

N/A

User interface changes

N/A

API changes

A new API that would convert timestamp to a date format, that can be passed to xmlrpc_date_get_xml() which would add the date correctly in the xml-rpc request.

Data model changes

N/A

CommentFileSizeAuthor
#11 interdiff-9-12.txt2.56 KBsubhojit777
#11 2954303-11.patch3.03 KBsubhojit777
#9 2954303-9.patch2.83 KBsubhojit777
#9 2954303-5-9.txt2.01 KBsubhojit777
#5 2954303-5.patch847 bytessubhojit777
#2 2954303-2.patch868 bytessubhojit777

Comments

subhojit777 created an issue. See original summary.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Active » Needs review
StatusFileSize
new 2954303-2.patch868 bytes

Status: Needs review » Needs work

The last submitted patch, 2: 2954303-2.patch, failed testing. View results

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented

I don't know why the patch fails to apply. Alex Pott confirmed that it is applying with no errors for him.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Needs work » Needs review
StatusFileSize
new 2954303-5.patch847 bytes

The last submitted patch, 2: 2954303-2.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 5: 2954303-5.patch, failed testing. View results

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Needs work » Needs review
subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
StatusFileSize
new 2954303-5-9.txt2.01 KB
new 2954303-9.patch2.83 KB
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work 
  1. +++ b/modules/simpletest/tests/xmlrpc.test
@@ -281,3 +281,57 @@ class XMLRPCMessagesTestCase extends DrupalWebTestCase {
+  protected function example() {

    date_dataprovider would be more in line of a name for a true PHP Unit test. Not sure if that means it is better, but I like it. 

  2. +++ b/modules/simpletest/tests/xmlrpc.test
@@ -281,3 +281,57 @@ class XMLRPCMessagesTestCase extends DrupalWebTestCase {
+    return array(

    To make this more readable, perhaps add an additional level of 'actual' and 'expected' in this array. Actual would be the unix timestamp. Then year/month/etc. would be keyed under expected.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Needs work » Needs review
StatusFileSize
new 2954303-11.patch3.03 KB
new interdiff-9-12.txt2.56 KB

Sounds good. Changes made.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs change record

This has great test coverage and adds an API function. So all that is missing is a change record for RTBC.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Reviewed & tested by the community » Needs work
subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Issue tags: -Needs change record

Done. https://www.drupal.org/node/2956129

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Needs work » Needs review
heddn’s picture

heddn
Primary language English
Location Illinois
commented

Can we show the before/after of what someone needed to do in the CR?

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented

I don't think so. The project I was working on, I was using the API, and the service kept throwing error, complaining that the data passed are incorrect. After debugging I found that Drupal was not passing the date in correct format, the datetime was not enclosed inside the dateTime.iso8601 xml tag in the request (see xmlrpc_date_get_xml()). And I figured the problem, and hence this API.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented

Sorry @heddn I misunderstood your request. I have updated the CR. Please check now.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Reviewed & tested by the community

Looking good. All feedback addressed.

pifagor’s picture

pifagor commented

Looks good for me.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Reviewed & tested by the community » Needs work

Small issue with the code itself.

+++ b/includes/common.inc
@@ -8492,3 +8492,27 @@ function drupal_get_filetransfer_info() {
+function drupal_get_xml_rpc_date($time = REQUEST_TIME) {

REQUEST_TIME is deprecated since https://www.drupal.org/node/2785211 . This function may be invoked at a point where the container is not available, though, so some degree of explanation for the default value actually used needs to be added in the comments if there is no way to use the time service.

Also, the patch does not include any use case for this function in the module itself, so why is it needed at all ?

Beyond that, can you explain why this change is needed per the XML-RPC spec at http://xmlrpc.scripting.com/spec.html or a specific compatibility suite test ?

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.