[#2] Encode POST data when not None

The underlying urllib library reacts weirdly when non-string values is passed through on certain versions of Python. For some reason passing a dict worked in Python 2.6 but not 2.7. From now on the definition of a something that should be a POST request is any data that is not None or False. Empty dict, list or string will become an empty string. I'd rather be consistent with an explicit way for all elements, instead of saying that an empty list will not be converted into a post while an empty string or dict will.
gaqzi · Aug 23, 2015 · defec42 · defec42
1 parent 4c270bf
commit defec42
Show file tree

Hide file tree

Showing 4 changed files with 159 additions and 3 deletions.
diff --git a/gocd/server.py b/gocd/server.py
@@ -88,7 +88,8 @@ def request(self, path, data=None, headers=None):
           path (str): The full path on the Go server to request.
             This includes any query string attributes.
           data (str, dict, optional): If any data is present this
-            request will become a POST request
+            request will become a POST request. Anything that is not
+            ``None`` or ``False`` will turn the request into a POST.
           headers (dict, optional): Headers to set for this particular
             request
 
@@ -174,11 +175,20 @@ def _request(self, path, data=None, headers=None):
         data = self._inject_authenticity_token(data, path)
         return urllib2.Request(
             self._url(path),
-            # GET is None, and anything that is a string is POST
-            data=urlencode(data) if data is not None else data,
+            data=self._encode_data(data),  # None or False == GET request
             headers=default_headers
         )
 
+    def _encode_data(self, data):
+        if isinstance(data, dict):
+            return urlencode(data)
+        elif isinstance(data, str):
+            return data
+        elif data is None or data is False:
+            return None
+        else:  # True, []
+            return ''
+
     def _url(self, path):
         return urljoin(self.host, path)
 

diff --git a/tests/fixtures/cassettes/server-data-for-get-requests.yml b/tests/fixtures/cassettes/server-data-for-get-requests.yml
@@ -0,0 +1,93 @@
+interactions:
+- request:
+    body: null
+    headers:
+      Connection: [close]
+      Host: ['localhost:8153']
+      User-Agent: [py-gocd]
+    method: GET
+    uri: http://localhost:8153/go/api/pipelines/Simple-with-lock/pause
+  response:
+    body: {string: !!python/unicode "<!DOCTYPE HTML>\n<!--[if lt IE 7]> <html xmlns=\"http://www.w3.org/1999/xhtml\"
+        lang=\"en\" class=\"no-js ie6 oldie\"> <![endif]-->\n<!--[if IE 7]> <html
+        xmlns=\"http://www.w3.org/1999/xhtml\" lang=\"en\" class=\"no-js ie7 oldie\">
+        <![endif]-->\n<!--[if IE 8]> <html xmlns=\"http://www.w3.org/1999/xhtml\"
+        lang=\"en\" class=\"no-js ie8 oldie\"> <![endif]-->\n<!--[if gt IE 8]><!-->\n<html
+        xmlns=\"http://www.w3.org/1999/xhtml\" lang='en'>\n<!--<![endif]-->\n\n\n<head>\n
+        \   <title>\n    HTTP ERROR 404 - Go\n</title>\n\n<link debug=\"false\" href=\"/go/assets/application-8f30e9364d6c69c71c8a76c50bc8e0ce.css\"
+        media=\"all\" rel=\"stylesheet\" />\n<link debug=\"false\" href=\"/go/assets/patterns/application-5a2cf49bd5c98c87f1f9f7a1c17b1131.css\"
+        media=\"all\" rel=\"stylesheet\" />\n\n<!-- START: prototype hooks for css
+        injection -->\n\n<!-- END: prototype hooks for css injection -->\n\n<script
+        debug=\"false\" src=\"/go/assets/application-ea0319c4737b736cc4da43f3f849f60b.js\"></script>\n<![if
+        !IE]>\n    <script src=\"/go/assets/lib/d3-3.1.5.min-18acdd172951a1c84db7dfac05912508.js\"></script>\n<![endif]>\n<!--[if
+        gt IE 8]><!-->\n    <script src=\"/go/assets/lib/d3-3.1.5.min-18acdd172951a1c84db7dfac05912508.js\"></script>\n<!--<![endif]-->\n\n\n<link
+        rel=\"shortcut icon\" href=\"/go/assets/cruise-a9724b6cd5ccc7da0532f027b0970936.ico\"/>\n</head>\n<body
+        id=\"application\" class=\"application\">\n<div id=\"body_bg\">\n    <div
+        id=\"header\">\n        <div class=\"header clear_float\">\n  <a href=\"/go/pipelines\"
+        id=\"application_logo\">&nbsp;</a>\n  <div class=\"application_nav\">\n  <input
+        id=\"server_timestamp\" name=\"server_time\" type=\"hidden\" value=\"1440336414\"
+        />        <ul class=\"user\">\n            <li class=\"help\">\n                <a
+        href=\"http://www.go.cd/documentation/user/current\" target=\"_blank\">Help</a>\n
+        \           </li>\n        </ul>\n\n    <ul class=\"tabs\">\n        <li id='cruise-header-tab-pipelines'
+        class=\"\">\n            <a href=\"/go/pipelines\">PIPELINES</a>        </li>\n
+        \       <li id='cruise-header-tab-environments' class=\"\">\n            <a
+        href=\"/go/environments\">ENVIRONMENTS</a>        </li>\n        <li id='cruise-header-tab-agents'
+        class=\"\">\n            <a href=\"/go/agents\">AGENTS</a>        </li>\n
+        \       <li id=\"cruise-header-tab-admin\" class=\"\">\n                <a
+        class=\"dropdown-arrow-icon\" data-toggle=\"dropdown\" href=\"#\">ADMIN</a>
+        \               <ul class=\"dropdown-menu\" role=\"menu\">\n                        <li
+        role=\"presentation\">\n    <a href=\"/go/admin/pipelines\">Pipelines</a></li>\n<li
+        role=\"presentation\">\n    <a href=\"/go/admin/templates\">Templates</a></li>\n<li
+        role=\"presentation\">\n    <a href=\"/go/admin/config_xml\">Config XML</a></li>\n<li
+        role=\"presentation\">\n    <a href=\"/go/admin/config/server\">Server Configuration</a></li>\n<li
+        role=\"presentation\">\n    <a href=\"/go/admin/users\">User Summary</a></li>\n<li
+        role=\"presentation\">\n    <a href=\"/go/oauth/admin/clients\">OAuth Clients</a></li>\n<li
+        role=\"presentation\">\n    <a href=\"/go/admin/gadgets/oauth_clients\">OAuth
+        Enabled Gadget Providers</a></li>\n<li role=\"presentation\">\n    <a href=\"/go/admin/backup\">Backup</a></li>\n<li
+        role=\"presentation\">\n    <a href=\"/go/admin/plugins\">Plugins</a></li>\n<li
+        role=\"presentation\">\n    <a href=\"/go/admin/package_repositories/new\">Package
+        Repositories</a></li>\n                </ul>\n        </li>\n    </ul>\n    <div
+        class=\"error_messaging_counter\">\n  <div id=\"cruise_message_counts\" class=\"cruise_messages\">\n
+        \ </div>\n  <div id=\"cruise_message_body\" style=\"display:none;\">\n  </div>\n\n
+        \    <script type=\"text/javascript\">\n        Util.on_load(function() {new
+        AjaxRefresher('/go/server/messages.json', null);});\n     </script>\n</div>\n</div>\n<div
+        class=\"back_to_top\" title=\"Scroll to Top\">Top</div>\n</div>\n    </div>\n\n
+        \   <div id='body_content'>\n        <div class=\"messaging_wrapper\" id=\"messaging_wrapper\">\n
+        \           <div class=\"flash\" id=\"message_pane\">\n</div>\n\n                    </div>\n
+        \               <div class=\"content_wrapper_outer\"><div class=\"content_wrapper_inner\">\n<div
+        class=\"notification pagenotfound\">\n    <div class='biggest'>:(</div>\n
+        \   <h3>The url [ /go/api/pipelines/Simple-with-lock/pause ] you are trying
+        to reach doesn't appear to be correct.</h3>\n    <span>Go to <a href=\"/go/pipelines\">Pipelines</a></span>\n</div>\n</div></div>\n\n\n<script
+        type=\"text/javascript\">\n    pageResizer();\n    function pageResizer()
+        {\n        var $pagenotfound = $j('.pagenotfound');\n        $pagenotfound.parent(
+        \".content_wrapper_inner\").height($j(window).height() - 160).css({'padding':0});\n
+        \       $pagenotfound.parents(\"#body_content\").css({paddingTop:46});\n\n
+        \   }\n    $j(window).resize(function() {\n        pageResizer();\n    });\n</script>\n
+        \   </div>\n\n    <div id='footer'>\n        <ul class=\"copyright\" onclick=\"dashboard_periodical_executer.stop();\">\n
+        \   <li>Copyright &copy; 2015 <a href=\"http://www.thoughtworks.com/products\"
+        target='_blank'>ThoughtWorks, Inc.</a> Licensed under <a href=\"http://www.apache.org/licenses/LICENSE-2.0\"
+        target=\"_blank\">Apache License, Version 2.0</a>. Go includes <a href=\"/go/NOTICE/cruise_notice_file.pdf\"
+        target=\"_blank\">third-party software</a>.</li>\n    <li class=\"last\">Go
+        Version: 15.1.0(1863-42e58c22aefd03)</li>\n</ul>\n<ul class=\"links\">\n    <li><a
+        href=\"/go/cctray.xml\">(cc) CCTray Feed</a></li>\n    <li><a href=\"http://www.go.cd/documentation/user/current/api/go_api.html\"
+        target=\"_blank\">APIs</a></li>\n    <li><a href=\"http://www.go.cd/community/plugins.html\"
+        target=\"_blank\">Plugins</a></li>\n    <li><a href=\"http://www.go.cd/community/resources.html\"
+        target=\"_blank\">Community</a></li>\n    <li><a href=\"/go/about\">Server
+        Details</a></li>\n    <li class=\"last\"><a href=\"http://www.go.cd/documentation/user/current\"
+        target=\"_blank\">Help</a></li>\n</ul>\n\n    </div>\n</div>\n</body>\n</html>\n"}
+    headers:
+      cache-control: [no-cache]
+      connection: [close]
+      content-type: [text/html; charset=utf-8]
+      expires: ['Thu, 01-Jan-1970 00:00:00 GMT']
+      server: [Jetty()]
+      set-cookie: ['JSESSIONID=9fk46nmlvtkh14zbgytdbb1ke;Path=/go;Expires=Sun, 06-Sep-2015
+          13:26:54 GMT']
+      x-content-type-options: [nosniff]
+      x-frame-options: [SAMEORIGIN]
+      x-request-id: [4ca78163-e72e-41c2-b948-1856ab9c1cda]
+      x-runtime: ['2.273000']
+      x-ua-compatible: [chrome=1]
+      x-xss-protection: [1; mode=block]
+    status: {code: 404, message: Not Found}
+version: 1
diff --git a/tests/fixtures/cassettes/server-data-for-post-requests.yml b/tests/fixtures/cassettes/server-data-for-post-requests.yml
@@ -0,0 +1,30 @@
+interactions:
+- request:
+    body: ''
+    headers:
+      Connection: [close]
+      Content-Length: ['0']
+      Content-Type: [application/x-www-form-urlencoded]
+      Host: ['localhost:8153']
+      User-Agent: [py-gocd]
+    method: POST
+    uri: http://localhost:8153/go/api/pipelines/Simple-with-lock/pause
+  response:
+    body: {string: !!python/unicode ' '}
+    headers:
+      cache-control: ['max-age=0, private, must-revalidate']
+      connection: [close]
+      content-type: [text/html; charset=utf-8]
+      etag: ['"7215ee9c7d9dc229d2921a40e899ec5f"']
+      expires: ['Thu, 01-Jan-1970 00:00:00 GMT']
+      server: [Jetty()]
+      set-cookie: ['JSESSIONID=1pwjxivtslb8t17s9d4zkgtvfx;Path=/go;Expires=Sun, 06-Sep-2015
+          13:29:01 GMT']
+      x-content-type-options: [nosniff]
+      x-frame-options: [SAMEORIGIN]
+      x-request-id: [deeeda6f-7078-4b20-aa87-04e30a358815]
+      x-runtime: ['0.012000']
+      x-ua-compatible: [chrome=1]
+      x-xss-protection: [1; mode=block]
+    status: {code: 200, message: OK}
+version: 1
diff --git a/tests/test_server.py b/tests/test_server.py
@@ -1,4 +1,5 @@
 import pytest
+import urllib2
 import vcr
 
 from gocd.server import Server
@@ -34,6 +35,28 @@ def test_post_request_without_argument(server, cassette_name):
     assert response.headers.type == 'text/html'
 
 
+@pytest.mark.parametrize('data', [{}, [], '', True])
+def test_request_with_all_kinds_of_falsey_values_that_should_be_post(server, data):
+    with vcr.use_cassette('tests/fixtures/cassettes/server-data-for-post-requests.yml'):
+        response = urllib2.urlopen(
+            server._request('go/api/pipelines/Simple-with-lock/pause', data=data)
+        )
+
+    assert response.code == 200
+    assert response.headers.type == 'text/html'
+    assert response.fp.read() == ' '
+
+
+@pytest.mark.parametrize('data', [None, False])
+def test_request_with_with_explicitly_no_post_data(server, data):
+    # This is meant to fail with a 404 since this endpoint is post only.
+    with vcr.use_cassette('tests/fixtures/cassettes/server-data-for-get-requests.yml'):
+        with pytest.raises(urllib2.HTTPError) as exc:
+            urllib2.urlopen(server._request('go/api/pipelines/Simple-with-lock/pause', data=data))
+
+    assert exc.value.code == 404
+
+
 @vcr.use_cassette('tests/fixtures/cassettes/post-with-argument.yml')
 def test_post_with_an_argument(server):
     response = server.post(