Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix :transport_options allowing non-typical values #277

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

jjcarstens
Copy link
Contributor

Work was previously done to support non-typical values in the :transport_options for ThousandIsland with 43d49c8.

However, if using SSL and including multiple key/value pairs with a non-typical, such as :inet6, it would still error because of an attempt to Keyword.delete/2 out the :otp_app key.

This fixes that with Enum.reject/2 to get the same behavior and still allow the list as expected by :ssl and :gen_tcp

Example Error

** (Mix) Could not start application nerves_hub: NervesHub.Application.start(:normal, []) returned an error: shutdown: failed to start child: NervesHubWeb.DeviceEndpoint
    ** (EXIT) shutdown: failed to start child: {NervesHubWeb.DeviceEndpoint, :https}
        ** (EXIT) an exception was raised:
            ** (FunctionClauseError) no function clause matching in Keyword.delete_key/2
                (elixir 1.15.7) lib/keyword.ex:719: Keyword.delete_key([:inet6, {:versions, [:"tlsv1.2"]}, {:verify, :verify_peer}, {:verify_fun, {&NervesHub.SSL.verify_fun/3, nil}}, {:fail_if_no_peer_cert, true}, {:keyfile, ~c"/Users/jonjon/code/nerves_hub_web/test/fixtures/ssl/device.nerves-hub.org-key.pem"}, {:certfile, ~c"/Users/jonjon/code/nerves_hub_web/test/fixtures/ssl/device.nerves-hub.org.pem"}, {:cacertfile, ~c"/Users/jonjon/code/nerves_hub_web/test/fixtures/ssl/ca.pem"}, {:ip, {0, 0, 0, 0}}, {:otp_app, :nerves_hub}, {:alpn_preferred_protocols, ["h2", "http/1.1"]}], :otp_app)
                (elixir 1.15.7) lib/keyword.ex:720: Keyword.delete_key/2
                (bandit 1.1.1) lib/bandit.ex:264: Bandit.start_link/1
                (stdlib 5.1.1) supervisor.erl:420: :supervisor.do_start_child_i/3
                (stdlib 5.1.1) supervisor.erl:406: :supervisor.do_start_child/2
                (stdlib 5.1.1) supervisor.erl:390: anonymous fn/3 in :supervisor.start_children/2
                (stdlib 5.1.1) supervisor.erl:1258: :supervisor.children_map/4
                (stdlib 5.1.1) supervisor.erl:350: :supervisor.init_children/2
                (stdlib 5.1.1) gen_server.erl:962: :gen_server.init_it/2
                (stdlib 5.1.1) gen_server.erl:917: :gen_server.init_it/6
                (stdlib 5.1.1) proc_lib.erl:241: :proc_lib.init_p_do_apply/3

Work was previously done to support non-typical values in the `:transport_options`
for ThousandIsland with mtrudel@43d49c8.

However, if using SSL and including multiple key/value pairs with a non-typical,
such as `:inet6`, it would still error because of an attempt to `Keyword.delete/2`
out the `:otp_app` key.

This fixes that with `Enum.reject/2` to get the same behavior and still allow
the list as expected by `:ssl` and `:gen_tcp`
@mtrudel
Copy link
Owner

mtrudel commented Dec 20, 2023

Excellent catch Jon! I'm just waiting for green CI and I'll merge and release this.

Thanks for the PR!

@mtrudel mtrudel merged commit 766dc92 into mtrudel:main Dec 20, 2023
27 checks passed
@mtrudel
Copy link
Owner

mtrudel commented Dec 20, 2023

Published as 1.1.2

@@ -261,7 +261,7 @@ defmodule Bandit do
{:ok, options} -> options
{:error, message} -> raise "Plug.SSL.configure/1 encountered error: #{message}"
end
|> Keyword.delete(:otp_app)
|> Enum.reject(&(is_tuple(&1) and elem(&1, 0) == :otp_app))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
|> Enum.reject(&(is_tuple(&1) and elem(&1, 0) == :otp_app))
|> Enum.reject(&match?({:otp_app, _}, &1))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants