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

Add support for UNIX sockets #224

Merged
merged 1 commit into from Apr 2, 2022

Conversation

ColonelThirtyTwo
Copy link
Contributor

Breaking change.

  • Implement a new module connection, which abstracts std::net types into ones that can use
    std::os::unix::net types on Unix platforms.
  • Change ServerConfig::addr to a new, abstracted type and add http_unix method.
    http and https methods are unchanged and should still work.
  • Request::remote_addr now returns Option<&SocketAddr>. This is Some for TCP servers and
    None for UNIX servers (since UNIX remote sockets are almost always unnamed).

TODO: test on Windows to make sure everything still works. Will do that this evening.

Breaking change.

* Implement a new module `connection`, which abstracts `std::net` types into ones that can use
  `std::os::unix::net` types on Unix platforms.
* Change `ServerConfig::addr` to a new, abstracted type and add `http_unix` method.
  `http` and `https` methods are unchanged and should still work.
* `Request::remote_addr` now returns `Option<&SocketAddr>`. This is `Some` for TCP servers and
  `None` for UNIX servers (since UNIX remote sockets are almost always unnamed).
Copy link
Member

@bradfier bradfier left a comment

Choose a reason for hiding this comment

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

LGTM, one comment about our excessive use of inline in this code but you're really following the convention so it's not something you need to change.

I'll merge this and work on a short migration guide for users of 0.11

ssl: Some(config),
})
}

#[cfg(unix)]
#[inline]
Copy link
Member

Choose a reason for hiding this comment

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

I know there are #[inline] scattered all over this project but they're really unnecessary, I should do a cleanup where I remove them...

@bradfier
Copy link
Member

bradfier commented Apr 2, 2022

As you note actually, we need to check this works on Windows before we do a release, I'll try and get to that this weekend.

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.

None yet

2 participants