Skip to content

Option to have http.server listen on a unix socket instead of TCP #8

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 34 commits into from
Closed

Option to have http.server listen on a unix socket instead of TCP #8

wants to merge 34 commits into from

Conversation

RyanSquared
Copy link
Contributor

Review (http://stackoverflow.com/questions/15716302/so-reuseaddr-and-af-unix) on whether SO_REUSEADDR should be kept?

@daurnimator
Copy link
Owner

  • cs.listen should only be called in a single location.
  • might as well pass all the arguments (i.e. v6only and reuseport) that the user passes in. If they use a path and reuseport that's their problem. There won't be any problems if you pass in host,port and path all in the one constructor.
  • I'd prefer you didn't use the host variable to store a path, it's just confusing.
  • You need to write tests
  • Need to update docs

@daurnimator
Copy link
Owner

Furthermore, if you're allowing binding unix sockets, you should probably add support for the cs.listen options:

  • .mode string:nil fchmod or chmod socket after creating UNIX domain socket
  • .mask string:nil set and restore umask when binding UNIX domain sockets
  • .unlink boolean:false unlink socket path before binding

daurnimator
daurnimator reviewed Feb 10, 2016
View reviewed changes
http/server.lua Outdated
assert(host or path, "need host or path")
local port = tbl.port
if port == nil then
if port == nil and tbl.host then
Copy link
Owner

Choose a reason for hiding this comment

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

Just use host instead of indexing again.

@daurnimator
Copy link
Owner

Blocked on #9

@daurnimator
Copy link
Owner

#9 was resolved.

Are you able to resolve my other comments?

@daurnimator daurnimator changed the title Optionally use path instead of host/port combination Option to have http.server listen on a unix socket instead of TCP Feb 17, 2016
daurnimator
daurnimator reviewed Mar 14, 2016
View reviewed changes
spec/server_spec.lua Outdated
stream:get_headers()
stream:shutdown()
s:shutdown()
print 'shutting down server // spy'
Copy link
Owner

Choose a reason for hiding this comment

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

please remove these prints.

RyanSquared and others added 18 commits March 23, 2016 23:56
@RyanSquared
http/server: Optionally use path instead of host/port combination
fe4b189
@RyanSquared
spec/server|http/client: UNIX domain sockets for client, and tests
067a641
@daurnimator
LICENSE: Update copyright year
1c8612c
@daurnimator
doc/index.md: Use plural
d92207a
@daurnimator
http/server: Keep a condition variable to be alerted of shutdown.
f931c2a 
Fixes #9
@daurnimator
doc/index: Document current http.server methods
18cac34
@daurnimator
README: Add luarocks install instuctions
0e959e4 
Remember to remove the --server option once released
@daurnimator
spec/request_spec: Put handle_redirect tests into separate tests
1311c35
@daurnimator
http/request: Add :clone method
fe62e33
@daurnimator
http/request: Implement :handle_redirect by :clone-ing then changing …
9d0892b 
…fields

Fixes issue where a custom host/port were not retained over a relative redirect
@daurnimator
spec/util_spec: Add tests for split_authority
c89b285
@daurnimator
spec/util_spec: Add tests for to_authority
01225a8
@daurnimator
spec/util_spec: Add test for decodeURI
e42b15c
@daurnimator
README: Remove TODO section, add Features, update Status
c6422f6
@daurnimator
http/h2_{connection,stream}: Don't rely on compat53 when in lua 5.3
823a7ce
@daurnimator
http/h1_stream: On close_when_done, remove self from pipeline *before…
a73acef 
…* shutting down, but signal other streams *after*
@daurnimator
http/h1_connection: get_next_incoming_stream now blocks until there i…
b62814d 
…s data available
@daurnimator
.travis.yml: Run luacheck
3083c2a
daurnimator and others added 16 commits March 23, 2016 23:57
@daurnimator
.travis.yml: Install hererocks out of tree (fixes luacheck failure)
415341d
@daurnimator
http/request: Update port on scheme change
2a12379
@daurnimator
.travis.yml: Add test for lua 5.3 without compat53 installed
f7475d0
@daurnimator
http/h1_stream: Don't allow server to send headers while stream is idle
08b14e7 
At that time:
  - request method is unknown
  - support for transfer-encodings is unknown
  - peer HTTP version is unknown
@daurnimator
spec/compat_prosody_spec: Add full end-to-end test
bc20d50
@daurnimator
.luacov: Add compat source subdir
ff04668
@daurnimator
spec/compat_prosody_spec: Add POST test
9ee6ead
@daurnimator
http/compat/prosody: Remove trailing semicolons
0caa077
@daurnimator
spec/compat_prosody_spec: Add duplicate headers to POST test
972f656
@daurnimator
examples/h2_streaming: Commit http2 streaming example
12c8c20
@daurnimator
.gitignore: Actually ignore luacov files
78914f1
@daurnimator
http/tls: Remove duplicated cipher
d601307 
See mpeterv/luacheck#53 (comment)
@daurnimator
http/h1_stream: Calculate stats_sent based on uncompressed size
96b8ae6
http/cilent.lua: Fix merge with master
9852189
spec/server_spec: Remove paths via finally()
85aef6f
http/server|spec/server_spec: add (pending) generation for contexts w…
80097bc 
…hen using UNIX domain sockets
@RyanSquared
Copy link
Contributor Author

Closing for reformatting commits, will create a new PR later.

@RyanSquared RyanSquared mentioned this pull request Mar 24, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RyanSquared @daurnimator