jakoch / PHPTracRPC

A PHP library to interact with the Trac Bugtracker API via remote procedure calls.
4 stars 2 forks source link

doRequest() mangles datetimes #14

Closed mpedrummer closed 8 years ago

mpedrummer commented 8 years ago

Inside doRequest(), the code does a json_encode, and then modifies the resulting string somewhat. What's the purpose of that?

// json_encode $this->_request
if (is_array($this->request) === true) {
        $this->request = json_encode(array_pop($this->request));

        $this->request = str_replace(':', ': ', $this->request);
        $this->request = str_replace(',', ', ', $this->request);
        $this->request = str_replace('[]', '{}', $this->request);
    }

The second line of code, replacing the ':' with ': ', modifies date values passed in, and results in an error from the Trac server.

'message' => 'JsonProtocolException details : Invalid datetime string (2016-03-16 08: 46: 29)',     'code' => -32700,     'name' => 'JSONRPCError'

If this conversion is just for prettification (commenting it out hasn't broken anything, yet), perhaps looking for '":', and replacing with '": ' would work just as well, without mangling the dates?

jakoch commented 8 years ago

What's the purpose of that?

I can't remember. Maybe pretty printing - maybe some edge case. Feel free to remove the line and send a PR.

(But i remember, that the last replace helps to handle empty arrays properly, see https://github.com/jakoch/PHPTracRPC/issues/2 The idea was to ditch replacing and use $this->request = json_encode(array_pop($this->request), JSON_FORCE_OBJECT); instead, but that didn't work either.)

mpedrummer commented 8 years ago

Fair enough. My code is full of "I can't remember" too :)

I'll submit a pull request sometime today.