-
Notifications
You must be signed in to change notification settings - Fork 47
feat(ethapi): add eth cors settings #1021
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,15 +52,15 @@ impl Display for SocketAddress { | |
| } | ||
| } | ||
|
|
||
| impl std::net::ToSocketAddrs for SocketAddress { | ||
| type Iter = <String as std::net::ToSocketAddrs>::Iter; | ||
| impl ToSocketAddrs for SocketAddress { | ||
| type Iter = <String as ToSocketAddrs>::Iter; | ||
|
|
||
| fn to_socket_addrs(&self) -> std::io::Result<Self::Iter> { | ||
| self.to_string().to_socket_addrs() | ||
| } | ||
| } | ||
|
|
||
| impl TryInto<std::net::SocketAddr> for SocketAddress { | ||
| impl TryInto<SocketAddr> for SocketAddress { | ||
|
||
| type Error = std::io::Error; | ||
|
|
||
| fn try_into(self) -> Result<SocketAddr, Self::Error> { | ||
|
|
@@ -328,7 +328,10 @@ impl Settings { | |
| .list_separator(",") // need to list keys explicitly below otherwise it can't pase simple `String` type | ||
| .with_list_parse_key("resolver.connection.external_addresses") | ||
| .with_list_parse_key("resolver.discovery.static_addresses") | ||
| .with_list_parse_key("resolver.membership.static_subnets"), | ||
| .with_list_parse_key("resolver.membership.static_subnets") | ||
| .with_list_parse_key("eth.cors.allowed_origins") | ||
| .with_list_parse_key("eth.cors.allowed_methods") | ||
| .with_list_parse_key("eth.cors.allowed_headers"), | ||
| )) | ||
| // Set the home directory based on what was passed to the CLI, | ||
| // so everything in the config can be relative to it. | ||
|
|
@@ -381,7 +384,7 @@ mod tests { | |
|
|
||
| use crate::DbCompaction; | ||
|
|
||
| use super::Settings; | ||
| use super::{ConfigError, Settings}; | ||
|
|
||
| fn try_parse_config(run_mode: &str) -> Result<Settings, config::ConfigError> { | ||
| let current_dir = PathBuf::from("."); | ||
|
|
@@ -422,21 +425,41 @@ mod tests { | |
| let settings = with_env_vars(vec![ | ||
| ("FM_RESOLVER__CONNECTION__EXTERNAL_ADDRESSES", "/ip4/198.51.100.0/tcp/4242/p2p/QmYyQSo1c1Ym7orWxLYvCrM2EmxFTANf8wXmmE7DWjhx5N,/ip6/2604:1380:2000:7a00::1/udp/4001/quic/p2p/QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb"), | ||
| ("FM_RESOLVER__DISCOVERY__STATIC_ADDRESSES", "/ip4/198.51.100.1/tcp/4242/p2p/QmYyQSo1c1Ym7orWxLYvCrM2EmxFTANf8wXmmE7DWjhx5N,/ip6/2604:1380:2000:7a00::2/udp/4001/quic/p2p/QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb"), | ||
| ("FM_RESOLVER__MEMBERSHIP__STATIC_SUBNETS", "/r314/f410fijl3evsntewwhqxy6cx5ijdq5qp5cjlocbgzgey,/r314/f410fwplxlims2wnigaha2gofgktue7hiusmttwridkq"), | ||
| ("FM_ETH__CORS__ALLOWED_ORIGINS", "https://example.com,https://www.example.org"), | ||
| ("FM_ETH__CORS__ALLOWED_METHODS", "GET,POST"), | ||
| ("FM_ETH__CORS__ALLOWED_HEADERS", "Accept,Content-Type"), | ||
| // Set a normal string key as well to make sure we have configured the library correctly and it doesn't try to parse everything as a list. | ||
| ("FM_RESOLVER__NETWORK__NETWORK_NAME", "test"), | ||
| ], || try_parse_config("")).unwrap(); | ||
|
|
||
| assert_eq!(settings.resolver.discovery.static_addresses.len(), 2); | ||
| assert_eq!(settings.resolver.connection.external_addresses.len(), 2); | ||
| assert_eq!(settings.resolver.discovery.static_addresses.len(), 2); | ||
| assert_eq!(settings.resolver.membership.static_subnets.len(), 2); | ||
| assert_eq!( | ||
| format!("{:?}", settings.eth.cors.allowed_origins), | ||
| "List([\"https://example.com\", \"https://www.example.org\"])" | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ugly, but no other way of testing this... there isn't a public API on |
||
| ); | ||
| assert_eq!( | ||
| format!("{:?}", settings.eth.cors.allowed_methods), | ||
| "Const(Some(\"GET,POST\"))" | ||
| ); | ||
| assert_eq!( | ||
| format!("{:?}", settings.eth.cors.allowed_headers), | ||
| "Const(Some(\"accept,content-type\"))" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_empty_comma_separated() { | ||
| let settings = with_env_vars( | ||
| vec![ | ||
| ("FM_RESOLVER__DISCOVERY__STATIC_ADDRESSES", ""), | ||
| ("FM_RESOLVER__CONNECTION__EXTERNAL_ADDRESSES", ""), | ||
| ("FM_RESOLVER__DISCOVERY__STATIC_ADDRESSES", ""), | ||
| ("FM_RESOLVER__MEMBERSHIP__STATIC_SUBNETS", ""), | ||
| ("FM_ETH__CORS__ALLOWED_ORIGINS", ""), | ||
| ("FM_ETH__CORS__ALLOWED_METHODS", ""), | ||
| ("FM_ETH__CORS__ALLOWED_HEADERS", ""), | ||
| ], | ||
| || try_parse_config(""), | ||
| ) | ||
|
|
@@ -445,6 +468,18 @@ mod tests { | |
| assert_eq!(settings.resolver.connection.external_addresses.len(), 0); | ||
| assert_eq!(settings.resolver.discovery.static_addresses.len(), 0); | ||
| assert_eq!(settings.resolver.membership.static_subnets.len(), 0); | ||
| assert_eq!( | ||
| format!("{:?}", settings.eth.cors.allowed_origins), | ||
| "List([])" | ||
| ); | ||
| assert_eq!( | ||
| format!("{:?}", settings.eth.cors.allowed_methods), | ||
| "Const(None)" | ||
| ); | ||
| assert_eq!( | ||
| format!("{:?}", settings.eth.cors.allowed_headers), | ||
| "Const(None)" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -471,4 +506,51 @@ mod tests { | |
| multiaddr!(Dns4("bar.ai"), Tcp(5678u16)) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_cors_origins_variants() { | ||
| // relative URL without a base | ||
| let settings = with_env_vars( | ||
| vec![("FM_ETH__CORS__ALLOWED_ORIGINS", "example.com")], | ||
| || try_parse_config(""), | ||
| ); | ||
| assert!( | ||
| matches!(settings, Err(ConfigError::Message(ref msg)) if msg == "relative URL without a base") | ||
| ); | ||
|
|
||
| // opaque origin | ||
| let settings = with_env_vars( | ||
| vec![( | ||
| "FM_ETH__CORS__ALLOWED_ORIGINS", | ||
| "javascript:console.log(\"invalid origin\")", | ||
| )], | ||
| || try_parse_config(""), | ||
| ); | ||
| assert!( | ||
| matches!(settings, Err(ConfigError::Message(ref msg)) if msg == "opaque origins are not allowed") | ||
| ); | ||
|
|
||
| // Allow all with "*" | ||
| let settings = with_env_vars(vec![("FM_ETH__CORS__ALLOWED_ORIGINS", "*")], || { | ||
| try_parse_config("") | ||
| }); | ||
| assert!(settings.is_ok()); | ||
|
|
||
| // IPv4 | ||
| let settings = with_env_vars( | ||
| vec![("FM_ETH__CORS__ALLOWED_ORIGINS", "http://192.0.2.1:1234")], | ||
| || try_parse_config(""), | ||
| ); | ||
| assert!(settings.is_ok()); | ||
|
|
||
| // IPv6 | ||
| let settings = with_env_vars( | ||
| vec![( | ||
| "FM_ETH__CORS__ALLOWED_ORIGINS", | ||
| "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:1234", | ||
| )], | ||
| || try_parse_config(""), | ||
| ); | ||
| assert!(settings.is_ok()); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left these empty by default because there is no way to override them to be empty using an env var (empty strings are ignored by the config list parser).