Skip to content

Conversation

@MSP-Greg
Copy link
Member

@MSP-Greg MSP-Greg commented Mar 3, 2021

Description

Closes #2526

Tests for abstract UNIXSockets exist in the updated test framework. Will open a PR for it after all the library file changes have been accepted. Tested both server & control binding...

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] or [ci skip] to the pull request title.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@nateberkopec nateberkopec added feature waiting-for-review Waiting on review from anyone labels Mar 6, 2021
@MSP-Greg MSP-Greg force-pushed the 00-issue-2526-abstract branch from acd9604 to 8abbee1 Compare March 15, 2021 16:09
@MSP-Greg MSP-Greg changed the title Support Linux's abstract sockets Add support for Linux's abstract sockets Mar 15, 2021
@MSP-Greg
Copy link
Member Author

Rebased and added two simple tests. The new test framework has a test or two creating hundreds of abstract unix clients...

@MSP-Greg MSP-Greg force-pushed the 00-issue-2526-abstract branch from 8abbee1 to 23173a1 Compare April 21, 2021 22:58
@MSP-Greg
Copy link
Member Author

MSP-Greg commented Apr 21, 2021

Thought I'd rebase this. Also, curl can be used to verify the PR. Compile the Puma repo, then

# assumes tcp 127.0.0.1:40000 is available, change as needed
bundle exec ruby -Ilib bin/puma -q -w2 -t5:5 -b tcp://127.0.0.1:40000 test/rackup/realistic_response.ru
curl -kso /dev/null -w '%{size_download}' http://127.0.0.1:40000

bundle exec ruby -Ilib bin/puma -q -w2 -t5:5 -b unix://@puma.unix test/rackup/realistic_response.ru
curl -kso /dev/null -w '%{size_download}' --abstract-unix-socket puma.unix http:

The file realistic_response.ru actually returns a 200,000 byte body. Both curl commands should output 200000, which is the body byte size...

end

s = UNIXServer.new(path)
s = UNIXServer.new path.sub(/\A@/, "\0")
Copy link
Member

Choose a reason for hiding this comment

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

Everywhere that we check for a socket starting with @, I think we should have a comment explaining this a check for an abstract socket.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@nateberkopec nateberkopec added waiting-for-changes Waiting on changes from the requestor and removed waiting-for-review Waiting on review from anyone labels Apr 24, 2021
@nateberkopec
Copy link
Member

Minor style concerns, otherwise LGTM

@nateberkopec nateberkopec added this to the 5.3.0 milestone Apr 24, 2021
@MSP-Greg MSP-Greg force-pushed the 00-issue-2526-abstract branch from 23173a1 to 8d51591 Compare April 24, 2021 15:58
@nateberkopec nateberkopec merged commit fac83ae into puma:master Apr 24, 2021
@MSP-Greg MSP-Greg deleted the 00-issue-2526-abstract branch November 2, 2021 19:06
JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this pull request Sep 9, 2022
* Support Linux's abstract sockets

Closes puma#2526

* Add two simple UNIXSocket tests
MayCXC added a commit to MayCXC/puma that referenced this pull request Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature waiting-for-changes Waiting on changes from the requestor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for Linux's abstract sockets

2 participants