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
This commit is contained in:
parent
81ec48c7b4
commit
4e6c113bf0
7 changed files with 50 additions and 11 deletions
|
@ -1128,8 +1128,6 @@ class Request:
|
||||||
urllib_request.install_opener(opener)
|
urllib_request.install_opener(opener)
|
||||||
|
|
||||||
data = to_bytes(data, nonstring='passthru')
|
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)
|
request = RequestWithMethod(url, method, data)
|
||||||
|
|
||||||
# add the custom agent header, to help prevent issues
|
# add the custom agent header, to help prevent issues
|
||||||
|
|
|
@ -61,9 +61,10 @@ options:
|
||||||
version_added: "2.0"
|
version_added: "2.0"
|
||||||
method:
|
method:
|
||||||
description:
|
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
|
type: str
|
||||||
choices: [ CONNECT, DELETE, GET, HEAD, OPTIONS, PATCH, POST, PUT, REFRESH, TRACE ]
|
|
||||||
default: GET
|
default: GET
|
||||||
return_content:
|
return_content:
|
||||||
description:
|
description:
|
||||||
|
@ -303,6 +304,7 @@ import cgi
|
||||||
import datetime
|
import datetime
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
|
import re
|
||||||
import shutil
|
import shutil
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
|
@ -516,7 +518,7 @@ def main():
|
||||||
body=dict(type='raw'),
|
body=dict(type='raw'),
|
||||||
body_format=dict(type='str', default='raw', choices=['form-urlencoded', 'json', 'raw']),
|
body_format=dict(type='str', default='raw', choices=['form-urlencoded', 'json', 'raw']),
|
||||||
src=dict(type='path'),
|
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),
|
return_content=dict(type='bool', default=False),
|
||||||
follow_redirects=dict(type='str', default='safe', choices=['all', 'no', 'none', 'safe', 'urllib2', 'yes']),
|
follow_redirects=dict(type='str', default='safe', choices=['all', 'no', 'none', 'safe', 'urllib2', 'yes']),
|
||||||
creates=dict(type='path'),
|
creates=dict(type='path'),
|
||||||
|
@ -538,7 +540,7 @@ def main():
|
||||||
url = module.params['url']
|
url = module.params['url']
|
||||||
body = module.params['body']
|
body = module.params['body']
|
||||||
body_format = module.params['body_format'].lower()
|
body_format = module.params['body_format'].lower()
|
||||||
method = module.params['method']
|
method = module.params['method'].upper()
|
||||||
dest = module.params['dest']
|
dest = module.params['dest']
|
||||||
return_content = module.params['return_content']
|
return_content = module.params['return_content']
|
||||||
creates = module.params['creates']
|
creates = module.params['creates']
|
||||||
|
@ -548,6 +550,9 @@ def main():
|
||||||
|
|
||||||
dict_headers = module.params['headers']
|
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':
|
if body_format == 'json':
|
||||||
# Encode the body unless its a string, then assume it is pre-formatted JSON
|
# Encode the body unless its a string, then assume it is pre-formatted JSON
|
||||||
if not isinstance(body, string_types):
|
if not isinstance(body, string_types):
|
||||||
|
|
|
@ -15,7 +15,6 @@ $spec = @{
|
||||||
method = @{
|
method = @{
|
||||||
type = "str"
|
type = "str"
|
||||||
default = "GET"
|
default = "GET"
|
||||||
choices = "CONNECT", "DELETE", "GET", "HEAD", "MERGE", "OPTIONS", "PATCH", "POST", "PUT", "REFRESH", "TRACE"
|
|
||||||
}
|
}
|
||||||
content_type = @{ type = "str" }
|
content_type = @{ type = "str" }
|
||||||
headers = @{ type = "dict" }
|
headers = @{ type = "dict" }
|
||||||
|
@ -44,7 +43,7 @@ $spec = @{
|
||||||
$module = [Ansible.Basic.AnsibleModule]::Create($args, $spec)
|
$module = [Ansible.Basic.AnsibleModule]::Create($args, $spec)
|
||||||
|
|
||||||
$url = $module.Params.url
|
$url = $module.Params.url
|
||||||
$method = $module.Params.method
|
$method = $module.Params.method.ToUpper()
|
||||||
$content_type = $module.Params.content_type
|
$content_type = $module.Params.content_type
|
||||||
$headers = $module.Params.headers
|
$headers = $module.Params.headers
|
||||||
$body = $module.Params.body
|
$body = $module.Params.body
|
||||||
|
@ -68,6 +67,10 @@ $JSON_CANDIDATES = @('text', 'json', 'javascript')
|
||||||
$module.Result.elapsed = 0
|
$module.Result.elapsed = 0
|
||||||
$module.Result.url = $url
|
$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)) {
|
if ($creates -and (Test-AnsiblePath -Path $creates)) {
|
||||||
$module.Result.skipped = $true
|
$module.Result.skipped = $true
|
||||||
$module.Result.msg = "The 'creates' file or directory ($creates) already exists."
|
$module.Result.msg = "The 'creates' file or directory ($creates) already exists."
|
||||||
|
|
|
@ -28,7 +28,6 @@ options:
|
||||||
description:
|
description:
|
||||||
- The HTTP Method of the request or response.
|
- The HTTP Method of the request or response.
|
||||||
type: str
|
type: str
|
||||||
choices: [ CONNECT, DELETE, GET, HEAD, MERGE, OPTIONS, PATCH, POST, PUT, REFRESH, TRACE ]
|
|
||||||
default: GET
|
default: GET
|
||||||
content_type:
|
content_type:
|
||||||
description:
|
description:
|
||||||
|
|
|
@ -390,6 +390,20 @@
|
||||||
- result is failure
|
- result is failure
|
||||||
- "'failed to parse body as form_urlencoded: too many values to unpack' in result.msg"
|
- "'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
|
- name: Test client cert auth, no certs
|
||||||
uri:
|
uri:
|
||||||
url: "https://ansible.http.tests/ssl_client_verify"
|
url: "https://ansible.http.tests/ssl_client_verify"
|
||||||
|
|
|
@ -336,6 +336,20 @@
|
||||||
- get_custom_header.json.headers['Test-Header'] == 'hello'
|
- get_custom_header.json.headers['Test-Header'] == 'hello'
|
||||||
- get_custom_header.json.headers['Another-Header'] == 'world'
|
- 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
|
# client cert auth tests
|
||||||
|
|
||||||
- name: get request with timeout
|
- name: get request with timeout
|
||||||
|
|
|
@ -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):
|
def test_Request_open_invalid_method(urlopen_mock, install_opener_mock):
|
||||||
with pytest.raises(ConnectionError):
|
r = Request().open('UNKNOWN', 'https://ansible.com/')
|
||||||
r = Request().open('BOGUS', '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):
|
def test_Request_open_custom_method(urlopen_mock, install_opener_mock):
|
||||||
|
|
Loading…
Reference in a new issue