contribsys / faktory_worker_ruby

Faktory worker for Ruby
GNU Lesser General Public License v3.0
214 stars 31 forks source link

Fix password hashing when server has password enableb. fixes #68 #69

Closed ppires closed 2 years ago

ppires commented 2 years ago

Fixes #68

ppires commented 2 years ago

@mperham The fix was very simple, but the test no so much.

I noticed that the test expect that the Faktory server is already running in the machine (did I missed something?). If I start a server with passwrod enabled all tests fail except for the one I wrote. If the server has no password, only my test fails.

Do you have any suggestions to solve this problem?

mperham commented 2 years ago

Unfortunately the test suite is somewhat simple. It expects the server to be running with no password so jobs can be tested against a live server. We can change that to use a password by default so this code path is tested.

mperham commented 2 years ago

It's also possible that we just don't test this. The fact that you've tested it manually is sufficient.

mperham commented 2 years ago

See the test suite failure.

ppires commented 2 years ago

I initially was thinking something like this:

def with_faktory(password = nil)
  if (password)
    # system call to start faktory server with `password``
  else
    # system call to start faktory server without a password
  end
  yield
  # system call to get container ID and kill container
end

But this would slow down the test suite.

It's also possible that we just don't test this. The fact that you've tested it manually is sufficient.

For now I think this it is the fastest way to merge this PR.

mperham commented 2 years ago

Ugh, this is way harder than it should be. I think you want CGI.unescape. This is not HTML form data but data that goes in a URI, like a CGI parameter.