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

Minimal verification on URL-defined queue #412

Closed
maschwenk opened this issue Jul 19, 2017 · 4 comments
Closed

Minimal verification on URL-defined queue #412

maschwenk opened this issue Jul 19, 2017 · 4 comments

Comments

@maschwenk
Copy link
Contributor

maschwenk commented Jul 19, 2017

In the queue initialization there is no verification done on the queue if it is supplied as a URL. Is there some basic verification that could be done here to align it more with the name verification?

Perhaps just check a queue attribute? Or some other minimal operation that can verify this is valid?

https://github.com/phstc/shoryuken/blame/master/lib/shoryuken/queue.rb#L61

def set_name(name)
  self.name = name
  self.url  = client.get_queue_url(queue_name: name).queue_url
rescue Aws::Errors::NoSuchEndpointError, Aws::SQS::Errors::NonExistentQueue => ex
  raise ex, "The specified queue #{name} does not exist."
end

def set_url(url)
  self.name = url.split('/').last
  self.url  = url
end
@phstc
Copy link
Collaborator

phstc commented Jul 20, 2017

@maschwenk it could anticipate this queue_attributes , which would test the URL.

Could you share your use case for validating user-supplied URLs?

@maschwenk
Copy link
Contributor Author

maschwenk commented Jul 20, 2017

@phstc if you boot Shoryuken with an invalid URL (i.e. https://sqs.us-east-1.amazonaws.com/18290711111/my-dog-is-a-cat), you'll notice that Shoryuken doesn't complain at all, it silently sits idly. Whereas, if you supply a bad queue name, as you can see in snippet above, it immediately calls get_queue_url and that fails fast.

@phstc
Copy link
Collaborator

phstc commented Jul 20, 2017

@maschwenk I think something like this:

def set_name(name)
  self.name = name
  self.url  = client.get_queue_url(queue_name: name).queue_url
end

def set_url(url)
  self.name = url.split('/').last
  self.url  = url
end

def set_name_and_url(name_or_url)
  if name_or_url.start_with?('https://sqs.')
    set_url(name_or_url)
    
    # anticipate the fifo? checker for validating the queue URL 
    return fifo?
  end
  
  set_name(name_or_url)
rescue Aws::Errors::NoSuchEndpointError, Aws::SQS::Errors::NonExistentQueue => ex
  raise ex, "The specified queue #{name_or_url} does not exist."
end

Would do.

If you are willing to submit a PR that would great, otherwise I can work on that.

@maschwenk
Copy link
Contributor Author

@phstc I'd be happy to open a PR

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

No branches or pull requests

2 participants