Patchwork [V2,STABLE] phabricator: properly encode boolean types in the request body

login
register
mail settings
Submitter Matt Harbison
Date Dec. 23, 2018, 1:55 a.m.
Message ID <e9c606fef203621755d7.1545530111@Envy>
Download mbox | patch
Permalink /patch/37322/
State Accepted
Headers show

Comments

Matt Harbison - Dec. 23, 2018, 1:55 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1545431772 18000
#      Fri Dec 21 17:36:12 2018 -0500
# Node ID e9c606fef203621755d75b0434574a8a60ffd0ff
# Parent  54c3b4bd01f24039fd8bc18d48d11c0f1ba7c633
phabricator: properly encode boolean types in the request body

I tripped over this playing with `hg debugcallconduit` to query for valid
reviewers.  If the JSON on stdin is written as 'True' or 'False', python
complains it isn't valid JSON.  If it's written as 'true' or 'false', it made it
to the server, but got kicked back with this:

    abort: Conduit Error (ERR-CONDUIT-CORE): Error while reading "isBot":
           Expected boolean (true or false), got something else.

The test isn't really relevant here (the code can be reverted, and it will
pass), but this gives us coverage for the debug command.
Yuya Nishihara - Dec. 23, 2018, 2:30 a.m.
On Sat, 22 Dec 2018 20:55:11 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1545431772 18000
> #      Fri Dec 21 17:36:12 2018 -0500
> # Node ID e9c606fef203621755d75b0434574a8a60ffd0ff
> # Parent  54c3b4bd01f24039fd8bc18d48d11c0f1ba7c633
> phabricator: properly encode boolean types in the request body

Queued, thanks.

Patch

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -155,6 +155,8 @@  def urlencodenested(params):
     """
     flatparams = util.sortdict()
     def process(prefix, obj):
+        if isinstance(obj, bool):
+            obj = { True: b'true', False: b'false' }[obj]  # Python -> PHP form
         items = {list: enumerate, dict: lambda x: x.items()}.get(type(obj))
         if items is None:
             flatparams[prefix] = obj
diff --git a/tests/phabricator/phab-conduit.json b/tests/phabricator/phab-conduit.json
new file mode 100644
--- /dev/null
+++ b/tests/phabricator/phab-conduit.json
@@ -0,0 +1,73 @@ 
+{
+    "interactions": [
+        {
+            "response": {
+                "status": {
+                    "message": "OK", 
+                    "code": 200
+                }, 
+                "headers": {
+                    "content-type": [
+                        "application/json"
+                    ], 
+                    "date": [
+                        "Fri, 21 Dec 2018 22:19:11 GMT"
+                    ], 
+                    "x-content-type-options": [
+                        "nosniff"
+                    ], 
+                    "cache-control": [
+                        "no-store"
+                    ], 
+                    "strict-transport-security": [
+                        "max-age=0; includeSubdomains; preload"
+                    ], 
+                    "x-frame-options": [
+                        "Deny"
+                    ], 
+                    "set-cookie": [
+                        "phsid=A%2Fdv22bpksbdis3vfeksluagfslhfojblbnkro7we4; expires=Wed, 20-Dec-2023 22:19:11 GMT; Max-Age=157680000; path=/; domain=phab.mercurial-scm.org; secure; httponly"
+                    ], 
+                    "x-xss-protection": [
+                        "1; mode=block"
+                    ], 
+                    "expires": [
+                        "Sat, 01 Jan 2000 00:00:00 GMT"
+                    ], 
+                    "transfer-encoding": [
+                        "chunked"
+                    ], 
+                    "server": [
+                        "Apache/2.4.10 (Debian)"
+                    ]
+                }, 
+                "body": {
+                    "string": "{\"result\":{\"data\":[],\"maps\":{},\"query\":{\"queryKey\":null},\"cursor\":{\"limit\":100,\"after\":null,\"before\":null,\"order\":null}},\"error_code\":null,\"error_info\":null}"
+                }
+            }, 
+            "request": {
+                "method": "POST", 
+                "headers": {
+                    "accept": [
+                        "application/mercurial-0.1"
+                    ], 
+                    "content-type": [
+                        "application/x-www-form-urlencoded"
+                    ], 
+                    "content-length": [
+                        "70"
+                    ], 
+                    "host": [
+                        "phab.mercurial-scm.org"
+                    ], 
+                    "user-agent": [
+                        "mercurial/proto-1.0 (Mercurial 4.8.1+564-6f483b107eb5+20181221)"
+                    ]
+                }, 
+                "uri": "https://phab.mercurial-scm.org//api/user.search", 
+                "body": "constraints%5BisBot%5D=true&api.token=cli-hahayouwish"
+            }
+        }
+    ], 
+    "version": 1
+}
\ No newline at end of file
diff --git a/tests/test-phabricator.t b/tests/test-phabricator.t
--- a/tests/test-phabricator.t
+++ b/tests/test-phabricator.t
@@ -65,6 +65,27 @@  Create a differential diff:
   D4597 - created - 1a5640df7bbf: create beta for phabricator test
   saved backup bundle to $TESTTMP/repo/.hg/strip-backup/1a5640df7bbf-6daf3e6e-phabsend.hg
 
+  $ hg debugcallconduit user.search --test-vcr "$VCR/phab-conduit.json" <<EOF
+  > {
+  >     "constraints": {
+  >         "isBot": true
+  >     }
+  > }
+  > EOF
+  {
+    "cursor": {
+      "after": null,
+      "before": null,
+      "limit": 100,
+      "order": null
+    },
+    "data": [],
+    "maps": {},
+    "query": {
+      "queryKey": null
+    }
+  }
+
 Template keywords
   $ hg log -T'{rev} {phabreview|json}\n'
   1 {"id": "D4597", "url": "https://phab.mercurial-scm.org/D4597"}