From 4e6c113bf045948ecc605870a1fec18e8a5e3d3c Mon Sep 17 00:00:00 2001
From: Dag Wieers <dag@wieers.com>
Date: Thu, 28 Feb 2019 21:55:18 +0100
Subject: [PATCH] uri/win_uri: Make method a free text field (#49719)
* uri/win_uri: Make method a free text field
Since various interfaces introduce their own HTTP method (e.g. like
PROPFIND, LIST or TRACE) it's better to leave this up to the user.
* Fix HTTP method check in module_utils urls
* Add integration test for method UNKNOWN
* Clarify the change as requested during review
---
lib/ansible/module_utils/urls.py | 2 --
lib/ansible/modules/net_tools/basics/uri.py | 13 +++++++++----
lib/ansible/modules/windows/win_uri.ps1 | 7 +++++--
lib/ansible/modules/windows/win_uri.py | 1 -
test/integration/targets/uri/tasks/main.yml | 14 ++++++++++++++
test/integration/targets/win_uri/tasks/test.yml | 14 ++++++++++++++
test/units/module_utils/urls/test_Request.py | 10 ++++++++--
7 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py
index fd4130653bb..3ae97d6b0e2 100644
--- a/lib/ansible/module_utils/urls.py
+++ b/lib/ansible/module_utils/urls.py
@@ -1128,8 +1128,6 @@ class Request:
urllib_request.install_opener(opener)
data = to_bytes(data, nonstring='passthru')
- if method not in ('OPTIONS', 'GET', 'HEAD', 'POST', 'PUT', 'DELETE', 'TRACE', 'CONNECT', 'PATCH'):
- raise ConnectionError('invalid HTTP request method; %s' % method)
request = RequestWithMethod(url, method, data)
# add the custom agent header, to help prevent issues
diff --git a/lib/ansible/modules/net_tools/basics/uri.py b/lib/ansible/modules/net_tools/basics/uri.py
index 85483aa1426..a598b0168e1 100644
--- a/lib/ansible/modules/net_tools/basics/uri.py
+++ b/lib/ansible/modules/net_tools/basics/uri.py
@@ -61,9 +61,10 @@ options:
version_added: "2.0"
method:
description:
- - The HTTP method of the request or response. It MUST be uppercase.
+ - The HTTP method of the request or response.
+ - In more recent versions we do not restrict the method at the module level anymore
+ but it still must be a valid method accepted by the service handling the request.
type: str
- choices: [ CONNECT, DELETE, GET, HEAD, OPTIONS, PATCH, POST, PUT, REFRESH, TRACE ]
default: GET
return_content:
description:
@@ -303,6 +304,7 @@ import cgi
import datetime
import json
import os
+import re
import shutil
import sys
import tempfile
@@ -516,7 +518,7 @@ def main():
body=dict(type='raw'),
body_format=dict(type='str', default='raw', choices=['form-urlencoded', 'json', 'raw']),
src=dict(type='path'),
- method=dict(type='str', default='GET', choices=['CONNECT', 'DELETE', 'GET', 'HEAD', 'OPTIONS', 'PATCH', 'POST', 'PUT', 'REFRESH', 'TRACE']),
+ method=dict(type='str', default='GET'),
return_content=dict(type='bool', default=False),
follow_redirects=dict(type='str', default='safe', choices=['all', 'no', 'none', 'safe', 'urllib2', 'yes']),
creates=dict(type='path'),
@@ -538,7 +540,7 @@ def main():
url = module.params['url']
body = module.params['body']
body_format = module.params['body_format'].lower()
- method = module.params['method']
+ method = module.params['method'].upper()
dest = module.params['dest']
return_content = module.params['return_content']
creates = module.params['creates']
@@ -548,6 +550,9 @@ def main():
dict_headers = module.params['headers']
+ if not re.match('^[A-Z]+$', method):
+ module.fail_json(msg="Parameter 'method' needs to be a single word in uppercase, like GET or POST.")
+
if body_format == 'json':
# Encode the body unless its a string, then assume it is pre-formatted JSON
if not isinstance(body, string_types):
diff --git a/lib/ansible/modules/windows/win_uri.ps1 b/lib/ansible/modules/windows/win_uri.ps1
index 09dd55ba9ba..f1d2f3576c3 100644
--- a/lib/ansible/modules/windows/win_uri.ps1
+++ b/lib/ansible/modules/windows/win_uri.ps1
@@ -15,7 +15,6 @@ $spec = @{
method = @{
type = "str"
default = "GET"
- choices = "CONNECT", "DELETE", "GET", "HEAD", "MERGE", "OPTIONS", "PATCH", "POST", "PUT", "REFRESH", "TRACE"
}
content_type = @{ type = "str" }
headers = @{ type = "dict" }
@@ -44,7 +43,7 @@ $spec = @{
$module = [Ansible.Basic.AnsibleModule]::Create($args, $spec)
$url = $module.Params.url
-$method = $module.Params.method
+$method = $module.Params.method.ToUpper()
$content_type = $module.Params.content_type
$headers = $module.Params.headers
$body = $module.Params.body
@@ -68,6 +67,10 @@ $JSON_CANDIDATES = @('text', 'json', 'javascript')
$module.Result.elapsed = 0
$module.Result.url = $url
+if (-not ($method -cmatch '^[A-Z]+$')) {
+ $module.FailJson("Parameter 'method' needs to be a single word in uppercase, like GET or POST.")
+}
+
if ($creates -and (Test-AnsiblePath -Path $creates)) {
$module.Result.skipped = $true
$module.Result.msg = "The 'creates' file or directory ($creates) already exists."
diff --git a/lib/ansible/modules/windows/win_uri.py b/lib/ansible/modules/windows/win_uri.py
index b1bcbc61ef9..59e8efc9062 100644
--- a/lib/ansible/modules/windows/win_uri.py
+++ b/lib/ansible/modules/windows/win_uri.py
@@ -28,7 +28,6 @@ options:
description:
- The HTTP Method of the request or response.
type: str
- choices: [ CONNECT, DELETE, GET, HEAD, MERGE, OPTIONS, PATCH, POST, PUT, REFRESH, TRACE ]
default: GET
content_type:
description:
diff --git a/test/integration/targets/uri/tasks/main.yml b/test/integration/targets/uri/tasks/main.yml
index b76c772256a..701d40cb805 100644
--- a/test/integration/targets/uri/tasks/main.yml
+++ b/test/integration/targets/uri/tasks/main.yml
@@ -390,6 +390,20 @@
- result is failure
- "'failed to parse body as form_urlencoded: too many values to unpack' in result.msg"
+- name: Validate invalid method
+ uri:
+ url: https://{{ httpbin_host }}/anything
+ method: UNKNOWN
+ register: result
+ ignore_errors: yes
+
+- name: Assert invalid method fails
+ assert:
+ that:
+ - result is failure
+ - result.status == 405
+ - "'METHOD NOT ALLOWED' in result.msg"
+
- name: Test client cert auth, no certs
uri:
url: "https://ansible.http.tests/ssl_client_verify"
diff --git a/test/integration/targets/win_uri/tasks/test.yml b/test/integration/targets/win_uri/tasks/test.yml
index ebeda5e8b94..a23220b0f44 100644
--- a/test/integration/targets/win_uri/tasks/test.yml
+++ b/test/integration/targets/win_uri/tasks/test.yml
@@ -336,6 +336,20 @@
- get_custom_header.json.headers['Test-Header'] == 'hello'
- get_custom_header.json.headers['Another-Header'] == 'world'
+- name: Validate invalid method
+ win_uri:
+ url: https://{{ httpbin_host }}/anything
+ method: UNKNOWN
+ register: invalid_method
+ ignore_errors: yes
+
+- name: Assert invalid method fails
+ assert:
+ that:
+ - invalid_method is failure
+ - invalid_method.status_code == 405
+ - invalid_method.status_description == 'METHOD NOT ALLOWED'
+
# client cert auth tests
- name: get request with timeout
diff --git a/test/units/module_utils/urls/test_Request.py b/test/units/module_utils/urls/test_Request.py
index 9b69edd3785..53242f2a623 100644
--- a/test/units/module_utils/urls/test_Request.py
+++ b/test/units/module_utils/urls/test_Request.py
@@ -363,8 +363,14 @@ def test_Request_open_cookies(urlopen_mock, install_opener_mock):
def test_Request_open_invalid_method(urlopen_mock, install_opener_mock):
- with pytest.raises(ConnectionError):
- r = Request().open('BOGUS', 'https://ansible.com/')
+ r = Request().open('UNKNOWN', 'https://ansible.com/')
+
+ args = urlopen_mock.call_args[0]
+ req = args[0]
+
+ assert req.data is None
+ assert req.get_method() == 'UNKNOWN'
+ # assert r.status == 504
def test_Request_open_custom_method(urlopen_mock, install_opener_mock):