Skip to content
This repository has been archived by the owner on Feb 5, 2024. It is now read-only.

Add support for SSL in StreamClient #70

Open
wants to merge 58 commits into
base: master
Choose a base branch
from

Conversation

damiankloip
Copy link
Contributor

There is a TODO in the code too. This was blocking me being able to replicate from a local Drupal site to a couch instance behind a proxy, using SSL.

@timmillwood
Copy link
Contributor

@zorn-v How does SSL work without this?

I think this is a much needed change to the stream client.

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

I have couchdb 2 instance with let's encrypt certificate. First of all I tried to adopt php-on-couch - curl just not accept cert. BTW you need to set cafile on couchdb local.ini (with fullchain file) for proper curl behavior. But there was another problems...
Stream client just works :D With "http" schema.

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

And I check with wireshark - it was SSL

@damiankloip
Copy link
Contributor Author

@zorn-v Hmm, seems like we can have both here though? You are instead getting redirected to HTTPS? Just trying to understand here. Can you test with this change and let me know if it breaks?

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

I think stream_context_create is smart enought to determine proper protocol. But maybe it was stupid in the past. I test it on php7

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

But wait. My "converter" works on php5.6

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

Couchdb listen two different ports for http and https (5984 and 6984). I don't know erlang but basics...

@damiankloip
Copy link
Contributor Author

I know the problem here, we are using a proxy that uses HTTPs, so we need to add a port, otherwise the default port of 5984 will get added to requests, which we don't want in our particular case. This PR allows us to hit https:// without a port added. Otherwise I cannot send requests to 443. It will end up like 'http://my.server.com:443' which doesn't work too well, even if we did add the port.

This PR seems to allow us to do both, and doesn't hurt any existing usage?

@damiankloip
Copy link
Contributor Author

@zorn-v right, so the thing here is that we're using another proxy between :)

@damiankloip
Copy link
Contributor Author

@zorn-v so you are sending requests to 'http://your-server:6984' ?

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

i do not use proxy ) For couch maybe nginx.
Anyway - this PR does not sense.

@zorn-v so you are sending requests to 'http://your-server:6984' ?
Nope, but stream_context_create does :)

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

Browser != library (language etc.) what you use.

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

I use couchdb directly. Without proxy.

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

This PR allows us to hit https:// without a port added. Otherwise I cannot send requests to 443.

I don't think that is problem. Allocate machine for couchdb is over. 80 and 443 - website.

Otherwise I cannot send requests to 443.

Just make your url...
https://ololo.olo:443

@damiankloip
Copy link
Contributor Author

The StreamClient only allows you to generate a URL like http://ololo.olo:443

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

And this addres not handled as SSL ?
Аnd you have proper SSL instalation on that address and port ?

@zorn-v
Copy link

zorn-v commented Jan 26, 2017

Just in case - try to set "url" property.

BTW - I think you have not "admin party"....

@damiankloip
Copy link
Contributor Author

The url property gets parsed and the various components of it stored, see https://github.com/doctrine/couchdb-client/blob/master/lib/Doctrine/CouchDB/CouchDBClient.php#L77 , then it's hard coded in the requests again in the StreamClient, as mentioned above.

@damiankloip
Copy link
Contributor Author

And yes, server handles SSL correctly (AWS instances) - just seems to be that things like http://fdfdd.df:443 are contradictory and just plain wrong.

@zorn-v
Copy link

zorn-v commented Mar 28, 2017

With nginx as proxy (erlang sometimes starts handle ssl with errors) to couchdb this PR is needed. Sorry for prev misconception.

@zorn-v zorn-v mentioned this pull request Mar 31, 2017
@damiankloip
Copy link
Contributor Author

No worries, thanks for confirming @zorn-v !

Allow default headers to be set for clients
@damiankloip
Copy link
Contributor Author

I think github is getting confused with this rebased on the newer master branch.

@robsonvn Looks like above you merged this into your fork, then reverted, then reverted the revert nut looks like we still need this change in here? Did you have issues with it?

@robsonvn
Copy link
Contributor

@damiankloip sorry about the mess, I did not have any issue with this PR.

I merged this PR into my fork and accidentally merged into the doctrine/master before taking to a public discussion, then I created PR #86

@damiankloip
Copy link
Contributor Author

damiankloip commented Apr 30, 2018

@robsonvn Oh, I see. No worries! And really good to see you making some progress on this repo now! 🎉

Shall we close out this PR then? I did rebase, but might not be needed now if it's already included in #86

@zorn-v
Copy link

zorn-v commented Apr 30, 2018

This repo is abandoned I think...
I use relaxedws/couchdb fork when I need couchdb client

@robsonvn
Copy link
Contributor

@damiankloip let's see how #86 goes, maybe this PR still useful for the CouchDB 1.6.x branch.
@zorn-v were are going to make it active again.

@zorn-v
Copy link

zorn-v commented Apr 30, 2018

were are going to make it active again.

Good to know 👍

@zorn-v
Copy link

zorn-v commented Apr 30, 2018

But who will be merge PRs ?
If you did not agree with the owners then NOBODY 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants