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

[critical] UNIXServer delete important file if it cannot bind to it #3764

Closed
bew opened this issue Dec 24, 2016 · 3 comments · Fixed by #3771
Closed

[critical] UNIXServer delete important file if it cannot bind to it #3764

bew opened this issue Dec 24, 2016 · 3 comments · Fixed by #3771
Assignees
Milestone

Comments

@bew
Copy link
Contributor

bew commented Dec 24, 2016

While writing the test for #3763 I realized there is another critical bug :

require "socket"

def check_file(prefix, file)
  if File.exists? file
    puts "#{prefix} File exists"
  else
    puts "#{prefix} No file"
  end
end

path = "/tmp/important_file"

# create an important file
File.write path, "important content"

check_file "Touch/", path

# try to bind
begin
  UNIXServer.new path
rescue err
  puts "Error raised !"
  puts
  check_file "Server/", path
  puts "Error was: #{err.message}"
end

Output for master, Crystal 0.20.3 :

Using compiled compiler at .build/crystal
Touch/ File exists
Error raised !

Server/ No file
Error was: bind: Address already in use

It is caused by the close in UNIXServer#new L32
Which will delete the file if you cannot bind to it

@asterite
Copy link
Member

Yes, I think this is a bug. The file should only be deleted on close if it's successfully bound.

I also tried it in Ruby (rescue => err instead of rescue err) and it doesn't delete the file.

@ysbaddaden ysbaddaden self-assigned this Dec 24, 2016
@ysbaddaden
Copy link
Contributor

Yes. I'll fix that.

@ysbaddaden
Copy link
Contributor

@Bew78LesellB thanks for detecting this issue!

ysbaddaden added a commit to ysbaddaden/crystal that referenced this issue Dec 26, 2016
@asterite asterite added this to the 0.20.4 milestone Dec 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants