themattharris / tmhOAuth

An OAuth 1.0A library written in PHP
Apache License 2.0
855 stars 335 forks source link

DELETE not handled correctly #162

Closed mrtnmueller closed 10 years ago

mrtnmueller commented 11 years ago

When working with the Xing API I encountered a problem with tmhOAuth. I think I got the same problem on twitter, but there the DELETE can be replaced with a POST request so I didn't really care.

However, here is the problem: DELETE requests will never be sent. Instead, a GET request will be performed.

for example: $this->tmhOAuth->request('DELETE', $this->tmhOAuth->url('v1/activities/'.$artifactId), array('id' => $artifactId)); will produce the following request header:

GET /v1/activities/781656653_2093e3.json HTTP/1.1
User-Agent: tmhOAuth 0.8.2+SSL - //github.com/themattharris/tmhOAuth
Host: api.xing.com    ...........

The problem is in function curlit(). Here are 2 cases: GET and POST. For all other requests, CURLOPT_CUSTOMREQUEST is set, but the request method is wrongly set to an array ($this->request_settings['postfields']) and url parameters are not considered.

SOLUTION: Add another case to curlit():

case 'DELETE':
          if (isset($this->request_settings['querystring']))
          $this->request_settings['url'] = $this->request_settings['url'] . '?' . $this->request_settings['querystring'];
          curl_setopt($c, CURLOPT_CUSTOMREQUEST, 'DELETE');
       break;

Same will probably go for PUT, I didn't test it though.

mrtnmueller commented 11 years ago

hm, after a moment of thinking i claim changing

default:
        if (isset($this->request_settings['postfields'])){
        curl_setopt($c, CURLOPT_CUSTOMREQUEST, $this->request_settings['postfields']);

to

default:
          if (isset($this->request_settings['querystring']))
          $this->request_settings['url'] = $this->request_settings['url'] . '?' . $this->request_settings['querystring'];
          curl_setopt($c, CURLOPT_CUSTOMREQUEST, $this->request_settings['method']);

might actually solve all remaining cases.

themattharris commented 10 years ago

merged into 0.8.4 and master.