From f65a74067089778f57200ea7cd87384d4cbdc590 Mon Sep 17 00:00:00 2001 From: docJerem Date: Tue, 19 May 2026 14:36:52 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor=20/=20Central?= =?UTF-8?q?ise=20xmerl=5Fscan=20behind=20ExSaml.Core.Xml.SafeXml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a single hardening surface for parsing untrusted SAML/XML. `SafeXml.scan/1` and `scan!/1` are now the only call sites of `:xmerl_scan.string` in `lib/`; they always pass `allow_entities: false`, `namespace_conformant: true`, and `quiet: true`. Callers cannot weaken these options. The binary→charlist conversion is centralised in `SafeXml.scan/1` too, which removes the per-site duplication that #22 (UTF-8 fix) had to patch in four files — future encoding changes are now a one-file diff. Replace the four ad-hoc `:xmerl_scan.string(...)` invocations: * `ExSaml.Core.Binding.decode_response/2` (DEFLATE + non-DEFLATE paths) * `ExSaml.Core.Sp.decrypt_assertion/2` * `ExSaml.Metadata.parse/1` Behaviour preserved: * `Binding` / `Sp` callers still receive a raise on malformed input (via `scan!/1`) — the surrounding `extract_assertion/3` and friends keep their existing `rescue`/`catch` wrappers. * `Metadata.parse/1` still returns `{:error, invalid_xml_violation()}` on any parse failure or non-element root. Behaviour change worth noting: * `Binding.decode_response/2` now suppresses xmerl warnings on malformed payloads (`quiet: true`). Previously they were logged. Test coverage: * New `ExSaml.Core.Xml.SafeXmlTest` exercises well-formed XML, binary vs. charlist parity, UTF-8 content, malformed input, empty input, XXE (external entity), internal entity expansion, and the raising variant. Sets up steps 2–4 of the plan discussed on #15 (AST-based check, config invariants for app-side concerns, CI wiring of `mix security.check_release`). --- lib/ex_saml/core/binding.ex | 23 +++---- lib/ex_saml/core/sp.ex | 11 +--- lib/ex_saml/core/xml/safe_xml.ex | 57 ++++++++++++++++ lib/ex_saml/metadata.ex | 29 +++------ test/ex_saml/core/xml/safe_xml_test.exs | 86 +++++++++++++++++++++++++ 5 files changed, 166 insertions(+), 40 deletions(-) create mode 100644 lib/ex_saml/core/xml/safe_xml.ex create mode 100644 test/ex_saml/core/xml/safe_xml_test.exs diff --git a/lib/ex_saml/core/binding.ex b/lib/ex_saml/core/binding.ex index 2c89df1..05675e3 100644 --- a/lib/ex_saml/core/binding.ex +++ b/lib/ex_saml/core/binding.ex @@ -33,6 +33,8 @@ defmodule ExSaml.Core.Binding do configured with `use_redirect_for_req: true`. """ + alias ExSaml.Core.Xml.SafeXml + require Record Record.defrecord(:xmlElement, Record.extract(:xmlElement, from_lib: "xmerl/include/xmerl.hrl")) @@ -62,14 +64,10 @@ defmodule ExSaml.Core.Binding do """ @spec decode_response(binary(), binary()) :: xml() def decode_response(@deflate, saml_response) do - xml_data = - saml_response - |> :base64.decode() - |> :zlib.unzip() - |> :binary.bin_to_list() - - {xml, _} = :xmerl_scan.string(xml_data, namespace_conformant: true, allow_entities: false) - xml + saml_response + |> :base64.decode() + |> :zlib.unzip() + |> SafeXml.scan!() end def decode_response(_encoding, saml_response) do @@ -77,15 +75,14 @@ defmodule ExSaml.Core.Binding do xml_data = try do - :zlib.unzip(data) |> :binary.bin_to_list() + :zlib.unzip(data) rescue - _e -> :binary.bin_to_list(data) + _e -> data catch - _kind, _reason -> :binary.bin_to_list(data) + _kind, _reason -> data end - {xml, _} = :xmerl_scan.string(xml_data, namespace_conformant: true, allow_entities: false) - xml + SafeXml.scan!(xml_data) end @doc """ diff --git a/lib/ex_saml/core/sp.ex b/lib/ex_saml/core/sp.ex index f382448..756a703 100644 --- a/lib/ex_saml/core/sp.ex +++ b/lib/ex_saml/core/sp.ex @@ -32,7 +32,8 @@ defmodule ExSaml.Core.Sp do SpMetadata, Subject, Util, - Xml.Dsig + Xml.Dsig, + Xml.SafeXml } @type xml :: record(:xmlElement) @@ -516,13 +517,7 @@ defmodule ExSaml.Core.Sp do assertion_xml = block_decrypt(to_string(algorithm), symmetric_key, cipher_value) - {assertion, _} = - :xmerl_scan.string(:binary.bin_to_list(assertion_xml), - namespace_conformant: true, - allow_entities: false - ) - - assertion + SafeXml.scan!(assertion_xml) end defp decrypt_key_info(encrypted_data, key) do diff --git a/lib/ex_saml/core/xml/safe_xml.ex b/lib/ex_saml/core/xml/safe_xml.ex new file mode 100644 index 0000000..667320a --- /dev/null +++ b/lib/ex_saml/core/xml/safe_xml.ex @@ -0,0 +1,57 @@ +defmodule ExSaml.Core.Xml.SafeXml do + @moduledoc """ + Single entry point for parsing untrusted SAML/XML payloads with `:xmerl_scan`. + + All calls go through this module so that the XXE-safe options + (`allow_entities: false`, `namespace_conformant: true`) and the + binary→charlist conversion are applied **once**, in one place. Callers do + not pass scan options — they cannot weaken the defaults. + + Centralising the wrapper means a future hardening change (new xmerl option, + stricter limits, alternative parser) is a one-file diff. It also + centralises the binary→charlist conversion that previously had to be + patched at every call site (cf. UTF-8 SAMLResponse handling). + + ## Functions + + * `scan/1` returns `{:ok, root}` or `{:error, :invalid_xml}`. + * `scan!/1` returns the root element or raises `ArgumentError`. Use it + when the surrounding code already runs inside a `try`/`rescue` or + when a malformed payload should propagate as an exception. + """ + + @scan_opts [namespace_conformant: true, allow_entities: false, quiet: true] + + @type xml :: term() + + @doc """ + Parse `xml` with `:xmerl_scan` using the hardened option set. + + Accepts either a UTF-8 binary or a charlist. Returns the parsed root on + success or `{:error, :invalid_xml}` on any parse failure (`rescue`d + exception or `catch`-able exit). + """ + @spec scan(binary() | charlist()) :: {:ok, xml()} | {:error, :invalid_xml} + def scan(xml) when is_binary(xml), do: xml |> :binary.bin_to_list() |> scan() + + def scan(xml) when is_list(xml) do + {root, _rest} = :xmerl_scan.string(xml, @scan_opts) + {:ok, root} + rescue + _ -> {:error, :invalid_xml} + catch + _kind, _reason -> {:error, :invalid_xml} + end + + @doc """ + Same as `scan/1` but raises `ArgumentError` on parse failure and returns + the parsed root directly. + """ + @spec scan!(binary() | charlist()) :: xml() + def scan!(xml) do + case scan(xml) do + {:ok, root} -> root + {:error, reason} -> raise ArgumentError, "invalid XML: #{inspect(reason)}" + end + end +end diff --git a/lib/ex_saml/metadata.ex b/lib/ex_saml/metadata.ex index c83b1cb..fa8906f 100644 --- a/lib/ex_saml/metadata.ex +++ b/lib/ex_saml/metadata.ex @@ -32,6 +32,7 @@ defmodule ExSaml.Metadata do See `ExSaml.Metadata.ValidationResult` for the shape of the returned struct. """ + alias ExSaml.Core.Xml.SafeXml alias ExSaml.Metadata.ValidationResult require Record @@ -100,26 +101,16 @@ defmodule ExSaml.Metadata do # --------------------------------------------------------------------------- defp parse(xml) do - charlist = :binary.bin_to_list(xml) - - try do - {root, _rest} = - :xmerl_scan.string( - charlist, - namespace_conformant: true, - allow_entities: false, - quiet: true - ) - - if Record.is_record(root, :xmlElement) do - {:ok, root} - else + case SafeXml.scan(xml) do + {:ok, root} -> + if Record.is_record(root, :xmlElement) do + {:ok, root} + else + {:error, invalid_xml_violation()} + end + + {:error, _} -> {:error, invalid_xml_violation()} - end - rescue - _ -> {:error, invalid_xml_violation()} - catch - _kind, _reason -> {:error, invalid_xml_violation()} end end diff --git a/test/ex_saml/core/xml/safe_xml_test.exs b/test/ex_saml/core/xml/safe_xml_test.exs new file mode 100644 index 0000000..dfc9d42 --- /dev/null +++ b/test/ex_saml/core/xml/safe_xml_test.exs @@ -0,0 +1,86 @@ +defmodule ExSaml.Core.Xml.SafeXmlTest do + use ExUnit.Case, async: true + + alias ExSaml.Core.Xml.SafeXml + + require Record + + Record.defrecord(:xmlElement, Record.extract(:xmlElement, from_lib: "xmerl/include/xmerl.hrl")) + + describe "scan/1" do + test "parses a well-formed XML binary" do + assert {:ok, root} = SafeXml.scan("hello") + assert Record.is_record(root, :xmlElement) + assert xmlElement(root, :name) == :root + end + + test "parses a charlist input identically to a binary input" do + xml = "" + assert {:ok, from_binary} = SafeXml.scan(xml) + assert {:ok, from_charlist} = SafeXml.scan(String.to_charlist(xml)) + assert xmlElement(from_binary, :name) == xmlElement(from_charlist, :name) + end + + test "accepts UTF-8 multibyte content in element values" do + assert {:ok, _root} = SafeXml.scan("éàü — ✓") + end + + test "returns {:error, :invalid_xml} on malformed input" do + assert {:error, :invalid_xml} = SafeXml.scan("") + end + + test "returns {:error, :invalid_xml} on empty input" do + assert {:error, :invalid_xml} = SafeXml.scan("") + end + + test "rejects DOCTYPE with an external entity (XXE)" do + xxe = """ + + + ]> + &xxe; + """ + + assert {:error, :invalid_xml} = SafeXml.scan(xxe) + end + + test "rejects DOCTYPE with an internal entity expansion" do + doc = """ + + + ]> + &hello; + """ + + assert {:error, :invalid_xml} = SafeXml.scan(doc) + end + end + + describe "scan!/1" do + test "returns the root element on success" do + root = SafeXml.scan!("") + assert Record.is_record(root, :xmlElement) + assert xmlElement(root, :name) == :root + end + + test "raises ArgumentError on malformed input" do + assert_raise ArgumentError, ~r/invalid XML/, fn -> + SafeXml.scan!(" + ]> + &xxe; + """ + + assert_raise ArgumentError, ~r/invalid XML/, fn -> + SafeXml.scan!(xxe) + end + end + end +end From b9df6918197d9a6ce52bfce26cfa8ef24e5dac94 Mon Sep 17 00:00:00 2001 From: docJerem Date: Tue, 19 May 2026 15:12:34 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9C=A8=20Feature=20/=20Tuple-only=20API?= =?UTF-8?q?=20+=20:on=5Ferror=20hook=20for=20SafeXml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the raising variant. `SafeXml.scan/1` is now the single entry point and always returns `{:ok, xml()} | {:error, :invalid_xml}` — more composable in `with`/`case` chains, safer to plug into pipelines. Cascade the tuple through the callers so the error stays a value end to end (no rescue at site, no hidden raise to catch): * `Binding.decode_response/2` public spec is now `{:ok, xml()} | {:error, :invalid_xml}` (was `xml()`). * `Sp.decrypt_assertion/2` now returns `{:ok, xml()} | {:error, :invalid_xml | :decrypt_failed}`; `extract_assertion/3` consumes it via `with`. * `Helper.decode_saml_payload/2` replaces its `rescue` with a `case` on the tuple; the error term changes from a stringified `inspect(error)` to the atom reason (`{:invalid_response, reason}`) — no caller pattern-matches on the previous shape. Add a configurable error hook for SIEM / OCSF pipelines: config :ex_saml, :safe_xml, on_error: &MyApp.SIEM.report_xml_error/1 The arity-1 handler receives a context map `%{reason: :invalid_xml, kind: :error | :exit | :throw, payload: …}`. Any exception from the handler is swallowed — the handler can never mask the original parse failure or alter control flow. Tests updated: * `SafeXmlTest` drops the `scan!/1` cases, adds five `:on_error` cases (called once per failure, not called on success, swallows handler errors, no-op when unconfigured, no-op when value is not an arity-1 function). * `BindingTest` patterns the `{:ok, _}` shape across the 7 call sites (roundtrip + UTF-8 regression suite). --- lib/ex_saml/core/binding.ex | 6 +- lib/ex_saml/core/sp.ex | 23 ++++---- lib/ex_saml/core/xml/safe_xml.ex | 76 +++++++++++++++++++------ lib/ex_saml/helper.ex | 8 +-- test/ex_saml/core/binding_test.exs | 14 ++--- test/ex_saml/core/xml/safe_xml_test.exs | 73 +++++++++++++++++------- 6 files changed, 137 insertions(+), 63 deletions(-) diff --git a/lib/ex_saml/core/binding.ex b/lib/ex_saml/core/binding.ex index 05675e3..6857109 100644 --- a/lib/ex_saml/core/binding.ex +++ b/lib/ex_saml/core/binding.ex @@ -62,12 +62,12 @@ defmodule ExSaml.Core.Binding do unzip is attempted, falling back to the raw decoded data if decompression fails. The resulting XML string is then parsed with `:xmerl_scan`. """ - @spec decode_response(binary(), binary()) :: xml() + @spec decode_response(binary(), binary()) :: {:ok, xml()} | {:error, :invalid_xml} def decode_response(@deflate, saml_response) do saml_response |> :base64.decode() |> :zlib.unzip() - |> SafeXml.scan!() + |> SafeXml.scan() end def decode_response(_encoding, saml_response) do @@ -82,7 +82,7 @@ defmodule ExSaml.Core.Binding do _kind, _reason -> data end - SafeXml.scan!(xml_data) + SafeXml.scan(xml_data) end @doc """ diff --git a/lib/ex_saml/core/sp.ex b/lib/ex_saml/core/sp.ex index 756a703..a2b2357 100644 --- a/lib/ex_saml/core/sp.ex +++ b/lib/ex_saml/core/sp.ex @@ -299,18 +299,13 @@ defmodule ExSaml.Core.Sp do [{:namespace, ns}] ) do [encrypted] when Record.is_record(encrypted, :xmlElement) -> - try do - decrypted = decrypt_assertion(encrypted, sp) - true = Record.is_record(decrypted, :xmlElement) - - case :xmerl_xpath.string(~c"/saml:Assertion", decrypted, [{:namespace, ns}]) do - [a] -> {:ok, a} - _ -> {:error, :bad_assertion} - end - rescue + with {:ok, decrypted} <- decrypt_assertion(encrypted, sp), + true <- Record.is_record(decrypted, :xmlElement), + [assertion] <- + :xmerl_xpath.string(~c"/saml:Assertion", decrypted, [{:namespace, ns}]) do + {:ok, assertion} + else _ -> {:error, :bad_assertion} - catch - _, _ -> {:error, :bad_assertion} end _ -> @@ -517,7 +512,11 @@ defmodule ExSaml.Core.Sp do assertion_xml = block_decrypt(to_string(algorithm), symmetric_key, cipher_value) - SafeXml.scan!(assertion_xml) + SafeXml.scan(assertion_xml) + rescue + _ -> {:error, :decrypt_failed} + catch + _, _ -> {:error, :decrypt_failed} end defp decrypt_key_info(encrypted_data, key) do diff --git a/lib/ex_saml/core/xml/safe_xml.ex b/lib/ex_saml/core/xml/safe_xml.ex index 667320a..f1c1cdd 100644 --- a/lib/ex_saml/core/xml/safe_xml.ex +++ b/lib/ex_saml/core/xml/safe_xml.ex @@ -12,24 +12,45 @@ defmodule ExSaml.Core.Xml.SafeXml do centralises the binary→charlist conversion that previously had to be patched at every call site (cf. UTF-8 SAMLResponse handling). - ## Functions + ## Error reporting hook - * `scan/1` returns `{:ok, root}` or `{:error, :invalid_xml}`. - * `scan!/1` returns the root element or raises `ArgumentError`. Use it - when the surrounding code already runs inside a `try`/`rescue` or - when a malformed payload should propagate as an exception. + An arity-1 function may be configured to receive parse-failure context. + Typical use: forward to a SIEM / OCSF pipeline. + + config :ex_saml, :safe_xml, + on_error: &MyApp.SIEM.report_xml_error/1 + + The handler is invoked **after** the `{:error, :invalid_xml}` tuple is + produced and receives a map: + + %{ + reason: :invalid_xml, + kind: :error | :exit | :throw, + payload: Exception.t() | term() + } + + Any exception raised by the handler is silently caught — the handler must + never be able to mask the original parse failure or alter the caller's + control flow. """ @scan_opts [namespace_conformant: true, allow_entities: false, quiet: true] @type xml :: term() + @type error_context :: %{ + required(:reason) => :invalid_xml, + required(:kind) => :error | :exit | :throw, + required(:payload) => Exception.t() | term() + } @doc """ Parse `xml` with `:xmerl_scan` using the hardened option set. Accepts either a UTF-8 binary or a charlist. Returns the parsed root on - success or `{:error, :invalid_xml}` on any parse failure (`rescue`d - exception or `catch`-able exit). + success or `{:error, :invalid_xml}` on any parse failure. + + Invokes the configured `:on_error` hook (if any) before returning the + error tuple. """ @spec scan(binary() | charlist()) :: {:ok, xml()} | {:error, :invalid_xml} def scan(xml) when is_binary(xml), do: xml |> :binary.bin_to_list() |> scan() @@ -38,20 +59,39 @@ defmodule ExSaml.Core.Xml.SafeXml do {root, _rest} = :xmerl_scan.string(xml, @scan_opts) {:ok, root} rescue - _ -> {:error, :invalid_xml} + e -> + notify(%{reason: :invalid_xml, kind: :error, payload: e}) + {:error, :invalid_xml} catch - _kind, _reason -> {:error, :invalid_xml} + :exit, reason -> + notify(%{reason: :invalid_xml, kind: :exit, payload: reason}) + {:error, :invalid_xml} + + :throw, value -> + notify(%{reason: :invalid_xml, kind: :throw, payload: value}) + {:error, :invalid_xml} end - @doc """ - Same as `scan/1` but raises `ArgumentError` on parse failure and returns - the parsed root directly. - """ - @spec scan!(binary() | charlist()) :: xml() - def scan!(xml) do - case scan(xml) do - {:ok, root} -> root - {:error, reason} -> raise ArgumentError, "invalid XML: #{inspect(reason)}" + # --------------------------------------------------------------------------- + # Error reporting hook + # --------------------------------------------------------------------------- + + defp notify(context) do + case Application.get_env(:ex_saml, :safe_xml, [])[:on_error] do + fun when is_function(fun, 1) -> + safe_invoke(fun, context) + + _ -> + :ok end end + + defp safe_invoke(fun, context) do + fun.(context) + :ok + rescue + _ -> :ok + catch + _, _ -> :ok + end end diff --git a/lib/ex_saml/helper.ex b/lib/ex_saml/helper.ex index ab6f327..909baa3 100644 --- a/lib/ex_saml/helper.ex +++ b/lib/ex_saml/helper.ex @@ -108,9 +108,9 @@ defmodule ExSaml.Helper do end defp decode_saml_payload(saml_encoding, saml_payload) do - xml = Core.Binding.decode_response(saml_encoding, saml_payload) - {:ok, xml} - rescue - error -> {:error, {:invalid_response, "#{inspect(error)}"}} + case Core.Binding.decode_response(saml_encoding, saml_payload) do + {:ok, xml} -> {:ok, xml} + {:error, reason} -> {:error, {:invalid_response, reason}} + end end end diff --git a/test/ex_saml/core/binding_test.exs b/test/ex_saml/core/binding_test.exs index a36d155..f55a1d9 100644 --- a/test/ex_saml/core/binding_test.exs +++ b/test/ex_saml/core/binding_test.exs @@ -101,7 +101,7 @@ defmodule ExSaml.Core.BindingTest do [_, b64_payload] = Regex.run(~r/name="SAMLRequest" value="([^"]+)"/, html) # decode_response with nil encoding (non-deflate path) should parse it - decoded = Binding.decode_response("", b64_payload) + {:ok, decoded} = Binding.decode_response("", b64_payload) assert Record.is_record(decoded, :xmlElement) assert xmlElement(decoded, :name) == :AuthnRequest end @@ -170,7 +170,7 @@ defmodule ExSaml.Core.BindingTest do compressed = :zlib.zip(xml_str) b64 = Base.encode64(compressed) - decoded = + {:ok, decoded} = Binding.decode_response( "urn:oasis:names:tc:SAML:2.0:bindings:URL-Encoding:DEFLATE", b64 @@ -184,7 +184,7 @@ defmodule ExSaml.Core.BindingTest do xml_str = "hello" b64 = Base.encode64(xml_str) - decoded = Binding.decode_response("", b64) + {:ok, decoded} = Binding.decode_response("", b64) assert Record.is_record(decoded, :xmlElement) assert xmlElement(decoded, :name) == :Response @@ -234,7 +234,7 @@ defmodule ExSaml.Core.BindingTest do xml = "" <> sample <> "" b64 = Base.encode64(xml) - decoded = Binding.decode_response("", b64) + {:ok, decoded} = Binding.decode_response("", b64) assert Record.is_record(decoded, :xmlElement) assert xmlElement(decoded, :name) == :Response @@ -248,7 +248,7 @@ defmodule ExSaml.Core.BindingTest do xml = " attr <> "\">x" b64 = Base.encode64(xml) - decoded = Binding.decode_response("", b64) + {:ok, decoded} = Binding.decode_response("", b64) assert Record.is_record(decoded, :xmlElement) assert attribute_value(decoded, :label) == unquote(expected) @@ -261,7 +261,7 @@ defmodule ExSaml.Core.BindingTest do xml = "" <> sample <> "" b64 = xml |> :zlib.zip() |> Base.encode64() - decoded = + {:ok, decoded} = Binding.decode_response( "urn:oasis:names:tc:SAML:2.0:bindings:URL-Encoding:DEFLATE", b64 @@ -283,7 +283,7 @@ defmodule ExSaml.Core.BindingTest do "" b64 = Base.encode64(xml) - decoded = Binding.decode_response("", b64) + {:ok, decoded} = Binding.decode_response("", b64) assert Record.is_record(decoded, :xmlElement) assert xmlElement(decoded, :name) == :"samlp:Response" diff --git a/test/ex_saml/core/xml/safe_xml_test.exs b/test/ex_saml/core/xml/safe_xml_test.exs index dfc9d42..50a9e1d 100644 --- a/test/ex_saml/core/xml/safe_xml_test.exs +++ b/test/ex_saml/core/xml/safe_xml_test.exs @@ -1,5 +1,5 @@ defmodule ExSaml.Core.Xml.SafeXmlTest do - use ExUnit.Case, async: true + use ExUnit.Case, async: false alias ExSaml.Core.Xml.SafeXml @@ -7,6 +7,9 @@ defmodule ExSaml.Core.Xml.SafeXmlTest do Record.defrecord(:xmlElement, Record.extract(:xmlElement, from_lib: "xmerl/include/xmerl.hrl")) + # `async: false` because tests in the `on_error hook` describe mutate + # `Application.put_env(:ex_saml, :safe_xml, ...)`, which is process-global. + describe "scan/1" do test "parses a well-formed XML binary" do assert {:ok, root} = SafeXml.scan("hello") @@ -58,29 +61,61 @@ defmodule ExSaml.Core.Xml.SafeXmlTest do end end - describe "scan!/1" do - test "returns the root element on success" do - root = SafeXml.scan!("") - assert Record.is_record(root, :xmlElement) - assert xmlElement(root, :name) == :root + describe ":on_error hook" do + setup do + previous = Application.get_env(:ex_saml, :safe_xml) + + on_exit(fn -> + if previous do + Application.put_env(:ex_saml, :safe_xml, previous) + else + Application.delete_env(:ex_saml, :safe_xml) + end + end) + + :ok end - test "raises ArgumentError on malformed input" do - assert_raise ArgumentError, ~r/invalid XML/, fn -> - SafeXml.scan!(" send(test_pid, {:ctx, ctx}) end + ) + + assert {:error, :invalid_xml} = SafeXml.scan(" - ]> - &xxe; - """ + test "is not invoked on successful parses" do + test_pid = self() + Application.put_env(:ex_saml, :safe_xml, on_error: fn _ctx -> send(test_pid, :called) end) + + assert {:ok, _} = SafeXml.scan("") + + refute_receive :called, 50 + end + + test "swallows handler exceptions and still returns the error tuple" do + Application.put_env(:ex_saml, :safe_xml, on_error: fn _ctx -> raise "handler boom" end) + + assert {:error, :invalid_xml} = SafeXml.scan(" - SafeXml.scan!(xxe) - end + Application.put_env(:ex_saml, :safe_xml, on_error: fn -> :wrong_arity end) + assert {:error, :invalid_xml} = SafeXml.scan("