Skip to content

WEBRick: Util.create_listeners doesn't make use of logger param #356

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 2 commits into from
Closed

WEBRick: Util.create_listeners doesn't make use of logger param #356

wants to merge 2 commits into from

Conversation

@vipulnsward
Copy link

WEBRick: Util.create_listeners doesn't make use of logger param

@vipulnsward
Copy link
Author

cc @nahi

@drbrain
Copy link
Member

drbrain commented Jul 10, 2013

This change isn't backwards compatible. When creating a server that runs as root you create the listeners, drop privileges, then create a server with DoNotListen: true and give it your precreated listeners. The new required parameter breaks such usage.

Additionally, your patch doesn't seem to do anything with the logger in #create_listeners. Did I miss something?

@vipulnsward
Copy link
Author

Actually the patch removes the logger. Not make it required.

@drbrain
Copy link
Member

drbrain commented Jul 10, 2013

Oops, my apologies. I did not read carefully enough

@zzak
Copy link
Member

zzak commented Jul 11, 2013

@vipulnsward @drbrain Should the logger parameter be removed from Utils#create_listeners as well?

@vipulnsward
Copy link
Author

@zzak actually that too is removed.
I should be writing better PR messages, so the changes aren't being confused with.

@zzak
Copy link
Member

zzak commented Jul 11, 2013

@vipulnsward There are no tests for #create_listeners. Do you think you could add some?

@vipulnsward
Copy link
Author

sure @zzak

@zzak
Copy link
Member

zzak commented Oct 15, 2013

@vipulnsward Any update here? Would like to see this merged, if we have the blessing of @nahi

@vipulnsward
Copy link
Author

@zzak @nahi updated.

@hsbt hsbt closed this in 73fc0cc Jan 2, 2015
ayumin pushed a commit to ayumin/ruby that referenced this pull request Jan 4, 2015
@hsbt
* lib/webrick/utils.rb: removed unused argument variable.
cb07240 
  [fix rubyGH-356] Patch by @vipulnsward
* lib/webrick/server.rb: ditto.
* lib/webrick/ssl.rb: ditto.
* test/webrick/test_utils.rb: added test for WEBrick::Utils#create_listeners.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@49101 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
mmasaki pushed a commit to mmasaki/ruby that referenced this pull request May 30, 2015
@hsbt
* lib/webrick/utils.rb: removed unused argument variable.
10510e5 
  [fix rubyGH-356] Patch by @vipulnsward
* lib/webrick/server.rb: ditto.
* lib/webrick/ssl.rb: ditto.
* test/webrick/test_utils.rb: added test for WEBrick::Utils#create_listeners.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@49101 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
@eregon eregon mentioned this pull request Dec 16, 2015
Closed
51 tasks
kddnewton added a commit to kddnewton/ruby that referenced this pull request Aug 5, 2022
@kddnewton
Ensure an out operand is allocated (ruby#356)
10b4c10
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vipulnsward @drbrain @zzak