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

Fix various stuff #5

Merged
merged 3 commits into from Jul 19, 2013
Merged

Fix various stuff #5

merged 3 commits into from Jul 19, 2013

Conversation

catwell
Copy link
Contributor

@catwell catwell commented Jun 4, 2012

Note: the first commit disables broken nonce reuse for Digest Authentication.

A better solution would be to actually implement a working version of it, but since it's optional (it just reduces the number of round trips) I haven't done it yet.

@jmettraux
Copy link
Owner

OK, I will look into it later in the week and try to come up with tests that cover your changes.

Thanks,

@catwell
Copy link
Contributor Author

catwell commented Jul 19, 2013

Going over my "old" issues, I found this one.

Any news? Are you still maintaining this library?

@jmettraux
Copy link
Owner

Argh.

No, I don't have the time to maintain it anymore. For my newer stuff I tend to use net-http-persistent and nothing else.

Sorry about that.

I did put this pull request back (and then forgot it) because of the mention_digest_auth being removed. IIRC a few (one?) people is using rufus-verbs because of digest auth and this killing of digest_auth would, well, remove a feature that other client libs don't seem to have.

Would you like to take over?

@catwell
Copy link
Contributor Author

catwell commented Jul 19, 2013

It doesn't disable Digest Auth. It disables Digest Auth nonce reuse (i.e. if you send two requests you will do the 401/200 handshake twice). It disables it because it is currently broken so people cannot use Digest Auth correctly anyway.

I also used Rufus Verbs because of Digest Auth support. The problem with taking over is that I am not really a Rubyist. I joined a company which used Ruby 3 years ago so I ended up writing and maintaining some stuff in it but these days I don't use it much anymore.

So, I'm not sure: would it be better for me to take over rufus-verbs or to migrate the projects we have that use it to something else? I am on vacation for 2 weeks starting tonight so I will take that time to think about it :)

@jmettraux
Copy link
Owner

OK, let me put the blame on you: you communicated poorly on that one. You simply appeared to remove a feature with a tiny explanation that I read as "I don't need digest auth, so I remove it altogether, I don't care if it breaks tests or other people's dependent code".

Bad point for me: I should have re-read my code and noticed your PR wouldn't have removed the digest auth entirely. I should have communicated with you. It seemed a pain in the ass to negotiate with you, learn from you and then go on an modernize my test suite. Your pull request felt like a simple, cold, bullet in the digest auth's head.

My suggestion: use another [maintained] library and adapt it. You know enough about Ruby and digest auth.

I will flag this projected as "unmaintained"... Well, I could sleep a night on it too and maybe get back at modernizing it (especially the test suite) and bringing net-http-persistent in... I still think you could go on and adopt another library, that'd free me in the short term.

Sorry for not asking more explanations a year ago.

John

@catwell
Copy link
Contributor Author

catwell commented Jul 19, 2013

OK, that's a reasonable answer. Sorry for not communicating better on this. I thought what I said above about it being optional because it only reduced the number of round trips was clear.

Yes I know a lot about Digest Auth by now :) The spec has lots of sharp edges. I have had to fix it in various libraries including Rack and Python's Requests, and implement it from scratch in Lua. So I was shaking my head in agreement when I read this ("client support for Digest is abysmal"). I do not design APIs that use it for authentication anymore.

I am fine with you flagging rufus-verbs as unmaintained, I will migrate the few projects I have that still use it to another library. I know at least HTTParty supports Digest Auth, although I don't like its interface much. Or I'll just use a cURL wrapper.

Thank you for all the fish :) You still make awesome Open Source software.

jmettraux added a commit that referenced this pull request Jul 19, 2013
fix digest authentication issue
@jmettraux jmettraux merged commit 878c3e3 into jmettraux:master Jul 19, 2013
@jmettraux
Copy link
Owner

OK, I'll take some time to port the tests to rspec this week-end and to adapt to your patch.

Thanks for your help!

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

Successfully merging this pull request may close these issues.

None yet

2 participants