From 321707f6eb60bec514753ba44a0347d394922996 Mon Sep 17 00:00:00 2001 From: Patrick Berkeley Date: Wed, 23 Aug 2023 16:10:48 +0200 Subject: [PATCH 1/2] Normalizes request body by parsing as params, then converting to a list Before this change, if the request body was a list of params that were in a different order than the params in the cassette, the request body match would fail. As of OTP 26, map key order is not guaranteed, so request bodies that are created using maps can fail to match since the order of their keys is not idempotent. These changes convert the request body to a list of params and sort it before comparing it to the request body in the cassette. This ensures cassettes will be matched as long as their request bodies contain the same set of key-value pairs as the incoming request body. --- lib/exvcr/handler.ex | 9 ++++++++- test/handler_stub_mode_test.exs | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/exvcr/handler.ex b/lib/exvcr/handler.ex index 8097e1b..29ed0e7 100644 --- a/lib/exvcr/handler.ex +++ b/lib/exvcr/handler.ex @@ -170,12 +170,19 @@ defmodule ExVCR.Handler do pattern = Regex.compile!(Enum.at(match, 1)) Regex.match?(pattern, key_body) else - request_body == key_body + normalize_request_body(request_body) == normalize_request_body(key_body) end else true end end + + defp normalize_request_body(request_body) do + request_body + |> URI.decode_query() + |> Map.to_list() + |> Enum.sort_by(fn {key, _val} -> key end) + end defp get_response_from_server(request, recorder, record?) do adapter = ExVCR.Recorder.options(recorder)[:adapter] diff --git a/test/handler_stub_mode_test.exs b/test/handler_stub_mode_test.exs index 053538f..72098c3 100644 --- a/test/handler_stub_mode_test.exs +++ b/test/handler_stub_mode_test.exs @@ -39,6 +39,14 @@ defmodule ExVCR.Adapter.HandlerStubModeTest do end end + test "request_body matches as string" do + use_cassette :stub, [url: 'http://localhost', method: :post, request_body: "some-string", body: "Hello World"] do + {:ok, status_code, _headers, body} = :ibrowse.send_req('http://localhost', [], :post, 'some-string') + assert status_code == '200' + assert to_string(body) =~ ~r/Hello World/ + end + end + test "request_body matches as regex" do use_cassette :stub, [url: 'http://localhost', method: :post, request_body: "~r/param1/", body: "Hello World"] do {:ok, status_code, _headers, body} = :ibrowse.send_req('http://localhost', [], :post, 'param1=value1¶m2=value2') @@ -55,6 +63,22 @@ defmodule ExVCR.Adapter.HandlerStubModeTest do end end + test "request_body matches as unordered list of params" do + use_cassette :stub, [url: 'http://localhost', method: :post, request_body: "param1=10¶m3=30¶m2=20", body: "Hello World"] do + {:ok, status_code, _headers, body} = :ibrowse.send_req('http://localhost', [], :post, 'param2=20¶m1=10¶m3=30') + assert status_code == '200' + assert to_string(body) =~ ~r/Hello World/ + end + end + + test "request_body mismatches as unordered list of params" do + assert_raise ExVCR.InvalidRequestError, fn -> + use_cassette :stub, [url: 'http://localhost', method: :post, request_body: "param1=10¶m3=30¶m4=40", body: "Hello World"] do + {:ok, _status_code, _headers, _body} = :ibrowse.send_req('http://localhost', [], :post, 'param2=20¶m1=10¶m3=30') + end + end + end + test "request_body mismatch should raise error" do assert_raise ExVCR.InvalidRequestError, fn -> use_cassette :stub, [url: 'http://localhost', method: :post, request_body: '{"one" => 1}'] do From a1d9dbc1d35f391fe1242cc6caf3d6a1cd3b407e Mon Sep 17 00:00:00 2001 From: Patrick Berkeley Date: Thu, 24 Aug 2023 16:26:18 +0200 Subject: [PATCH 2/2] Normalizes url by parsing params, converting to a list, and sorting Before this change, if the url query params were in a different order than the url params in the cassette, the request body match would fail. As of OTP 26, map key order is not guaranteed, so url params that are created using maps can fail to match since the order of their keys is not idempotent. These changes convert the url params to a list and sort it before comparing it to the url in the cassette. This ensures cassettes will be matched as long as their url params contain the same set of key-value pairs as the incoming url params (and the rest of the url matches too). --- .../response_mocking_with_param.json | 2 +- .../different_query_params_on.json | 2 +- lib/exvcr/handler.ex | 23 +++++++++++++++---- test/handler_custom_mode_test.exs | 3 +-- test/handler_options_test.exs | 2 +- test/handler_stub_mode_test.exs | 8 +++++++ 6 files changed, 31 insertions(+), 9 deletions(-) diff --git a/fixture/custom_cassettes/response_mocking_with_param.json b/fixture/custom_cassettes/response_mocking_with_param.json index d56ed50..a2f5806 100644 --- a/fixture/custom_cassettes/response_mocking_with_param.json +++ b/fixture/custom_cassettes/response_mocking_with_param.json @@ -1,7 +1,7 @@ [ { "request": { - "url": "http://example.com?auth_token=123abc", + "url": "http://example.com?auth_token=123abc&another_param=456", "method": "get" }, "response": { diff --git a/fixture/vcr_cassettes/different_query_params_on.json b/fixture/vcr_cassettes/different_query_params_on.json index 4385fa5..abc0831 100644 --- a/fixture/vcr_cassettes/different_query_params_on.json +++ b/fixture/vcr_cassettes/different_query_params_on.json @@ -5,7 +5,7 @@ "headers": [], "method": "get", "options": [], - "url": "http://localhost:34006/server?p=3" + "url": "http://localhost:34006/server?p=3&q=string" }, "response": { "body": "test_response_before", diff --git a/lib/exvcr/handler.ex b/lib/exvcr/handler.ex index 29ed0e7..84e0b19 100644 --- a/lib/exvcr/handler.ex +++ b/lib/exvcr/handler.ex @@ -112,13 +112,13 @@ defmodule ExVCR.Handler do pattern = Regex.compile!(Enum.at(match, 1)) Regex.match?(pattern, key_url) else - request_url == key_url + normalize_url(request_url) == normalize_url(key_url) end else request_url = parse_url(request_url, recorder_options) key_url = parse_url(key_url, recorder_options) - request_url == key_url + normalize_url(request_url) == normalize_url(key_url) end end @@ -176,12 +176,27 @@ defmodule ExVCR.Handler do true end end - + + defp normalize_url(url) do + original_url = URI.parse(url) + + original_url + |> Map.put(:query, normalize_query(original_url.query)) + |> URI.to_string() + end + defp normalize_request_body(request_body) do - request_body + normalize_query(request_body) + end + + defp normalize_query(nil), do: nil + + defp normalize_query(query) do + query |> URI.decode_query() |> Map.to_list() |> Enum.sort_by(fn {key, _val} -> key end) + |> URI.encode_query() end defp get_response_from_server(request, recorder, record?) do diff --git a/test/handler_custom_mode_test.exs b/test/handler_custom_mode_test.exs index aaedfff..ea0bcb1 100644 --- a/test/handler_custom_mode_test.exs +++ b/test/handler_custom_mode_test.exs @@ -4,11 +4,10 @@ defmodule ExVCR.Adapter.HandlerCustomModeTest do test "query param match succeeds with custom mode" do use_cassette "response_mocking_with_param", custom: true do - HTTPotion.get("http://example.com?auth_token=123abc", []).body =~ ~r/Custom Response/ + HTTPotion.get("http://example.com?another_param=456&auth_token=123abc", []).body =~ ~r/Custom Response/ end end - test "custom with valid response" do use_cassette "response_mocking", custom: true do assert HTTPotion.get("http://example.com", []).body =~ ~r/Custom Response/ diff --git a/test/handler_options_test.exs b/test/handler_options_test.exs index f9823aa..c926c6d 100644 --- a/test/handler_options_test.exs +++ b/test/handler_options_test.exs @@ -17,7 +17,7 @@ defmodule ExVCR.Adapter.HandlerOptionsTest do test "specifying match_requests_on: [:query] matches query params" do use_cassette "different_query_params_on", match_requests_on: [:query] do HttpServer.start(path: "/server", port: @port, response: "test_response_before") - assert HTTPotion.get("#{@url}?p=3", []).body =~ ~r/test_response_before/ + assert HTTPotion.get("#{@url}?q=string&p=3", []).body =~ ~r/test_response_before/ HttpServer.stop(@port) # this method call should NOT be mocked as previous "test_response_before" response diff --git a/test/handler_stub_mode_test.exs b/test/handler_stub_mode_test.exs index 72098c3..0e519c1 100644 --- a/test/handler_stub_mode_test.exs +++ b/test/handler_stub_mode_test.exs @@ -31,6 +31,14 @@ defmodule ExVCR.Adapter.HandlerStubModeTest do end end + test "url matches as regardless of query param order" do + use_cassette :stub, [url: "http://localhost?param1=10¶m2=20¶m3=30"] do + {:ok, status_code, _headers, body} = :ibrowse.send_req('http://localhost?param3=30¶m1=10¶m2=20', [], :get) + assert status_code == '200' + assert to_string(body) =~ ~r/Hello World/ + end + end + test "url matches as regex" do use_cassette :stub, [url: "~r/.+/"] do {:ok, status_code, _headers, body} = :ibrowse.send_req('http://localhost', [], :get)