Serve from other IPs #142
Serve from other IPs #142
Conversation
Was confused why these tests were failing on my end. Turned out I had a service running on 8080 and had to kill it to get these tests to pass
src/lune/builtins/net/mod.rs
Outdated
| let address_pattern = Regex::new(r"(?:.*:\/\/)?(\d+\.\d+\.\d+\.\d+)(?::\d+)?").unwrap(); | ||
|
|
||
| let address: Ipv4Addr = match &config.address { | ||
| Some(addr) => { | ||
| let addr_str = addr.to_str()?; | ||
|
|
||
| if let Some(caps) = address_pattern.captures(addr_str) { | ||
| caps[1].parse::<Ipv4Addr>()? | ||
| } else { | ||
| return Err(LuaError::runtime(format!( | ||
| "IP address format is incorrect (expected an IP in the form 'http://0.0.0.0' or '0.0.0.0', got '{addr_str}')" | ||
| ))); | ||
| } | ||
| } | ||
| None => DEFAULT_IP_ADDRESS, | ||
| }; |
There was a problem hiding this comment.
I feel like maybe regex is a bit overkill here, if all we're really doing is stripping out the http:// prefix we should be able to trim it down to something like
let addr_str = addr.to_str()?;
addr_str.trim_start_matches("http://").parse()?and we leave the rest of the validation and error handling to the parse implementation that Ipv4Addr has. If you want to keep the nice descriptive error message maybe also .parse().map_err(|| LuaError::runtime(...))?
There was a problem hiding this comment.
Agreed, I was hesitant to add regex here. We need to at least trim http://, https://, and the port :xxxx from the address. I opted for regex since at the time it was the easiest way to capture the IP.
Would it be worthwhile to consider trimming any other protocols? The current pattern could match things like ssh://, ftp://, etc. but I can't see those actually being used.
There was a problem hiding this comment.
Since this is explicitly just an http server, I don't think we need to do any other protocols. I missed that port was included in that pattern - unsure about what to do about that, maybe we should just let the parse error there if port is given, since port must be given as net.serve first arg anyway?
|
Seems like the new test cases on Windows run forever, which is a bit strange.. |
I'm developing on macOS, so this will be a bit hard for me to test. Any suggestions on debugging that? |
I think manually calling |
This PR introduces a new
addressproperty tonet.serveto change which IP address to serve off ofUsage:
Resolves #120