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

Replace UUID with SecureRandom.uuid #693

Merged
merged 1 commit into from
Jun 5, 2015
Merged

Replace UUID with SecureRandom.uuid #693

merged 1 commit into from
Jun 5, 2015

Conversation

zcross
Copy link

@zcross zcross commented May 27, 2015

Savon uses the uuid gem which does some pretty heavy lifting to generate UUIDs (e.g., shelling out via systemu). This PR removes this dependency, using Ruby's own SecureRandom.uuid.

@zanker
Copy link

zanker commented May 27, 2015

From testing, it looks like it's not necessary to use the uuid gem, as long as Savon doesn't need to work on Ruby 1.8.7, which was EOL'd anyway.

http://www.w3.org/Submission/ws-addressing/ don't appear to require any particular format. The docs primarily reference that it's used to prevent replay attacks, which SecureRandom.uuid handles no problem.

@zcross
Copy link
Author

zcross commented May 27, 2015

Addendum: kudos to @zanker for pairing with me in debugging an issue that led to this proposed change.

@wyaeld
Copy link

wyaeld commented Jun 5, 2015

@tjarratt any thoughts on this?

@tjarratt
Copy link
Contributor

tjarratt commented Jun 5, 2015

Thanks for the ping @wyaeld -- I had almost forgot about this thread.

I'm a huge fan of not shelling out in ruby, so this pull request is awesome. Many thanks to you, @zcross and @zanker!

tjarratt added a commit that referenced this pull request Jun 5, 2015
@tjarratt tjarratt merged commit 8da1f3c into savonrb:master Jun 5, 2015
@tjarratt
Copy link
Contributor

tjarratt commented Jun 5, 2015

From the changes, it seems that there's a vested interest in getting a new version of Savon published?

@wyaeld
Copy link

wyaeld commented Jun 5, 2015

Yes please. About to start a new project

On Fri, Jun 5, 2015, 5:15 PM Tim Jarratt notifications@github.com wrote:

From the changes, it seems that there's a vested interest in getting a new
version of Savon published?


Reply to this email directly or view it on GitHub
#693 (comment).

@tjarratt
Copy link
Contributor

tjarratt commented Jun 5, 2015

I've just released Savon v2.11.1 -- enjoy!

@zcross
Copy link
Author

zcross commented Jun 5, 2015

Thanks!

@zcross zcross deleted the zcross/secure-random-over-uuid branch May 10, 2016 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants