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

[close #1327] Fix double port bind in Rails #1383

Merged
merged 1 commit into from Aug 2, 2017

Conversation

schneems
Copy link
Contributor

@schneems schneems commented Aug 2, 2017

The "default" configuration puma level is initialized with a "binds" of tcp://0.0.0.0:9292. Puma is designed to be able to bind to multiple ports.

When a :port is sent from Rails along with an empty user_supplied_options then the port is treated as a "default". This is merged in with the system defaults, and then later converted into a "binds" via calling config.port in the set_host_port_to_config method.

The bug comes due to the "level" of the configuration. Since both are being set on the same "level" the port call does not over-write the existing binds but instead prepends to the array. We can fix by ensuring that any binds in a given "level" are empty before setting it.

The "default" configuration puma level is initialized with a "binds" of `tcp://0.0.0.0:9292`. Puma is designed to be able to bind to multiple ports. 

When a `:port` is sent from Rails along with an empty `user_supplied_options` then the port is treated as a "default". This is merged in with the system defaults, and then later converted into a "binds" via calling `config.port` in the `set_host_port_to_config` method. 

The bug comes due to the "level" of the configuration. Since both are being set on the same "level" the `port` call does not over-write the existing binds but instead prepends to the array. We can fix by ensuring that any binds in a given "level" are empty before setting it.
@schneems schneems merged commit d31d68a into master Aug 2, 2017
@nateberkopec nateberkopec deleted the schneems/fix-double-binds branch November 17, 2017 19:19
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

1 participant