Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Alternate Scheme Support for Client #24

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

Alternate Scheme Support for Client #24

wants to merge 5 commits into from

Conversation

astephens25
Copy link

I wanted to use the client with SSL and found DNode very difficult to extend so I added the support directly. The parsing of the scheme argument to the connect method is a bit odd, but was need to maintain compatibility with existing calls.

The new Connection class is simply to perform buffered IO on the stream as required by various wrapper streams, but leave the raw IO enabled for UDP (and TCP just to change as little as possible).

The use case for these changes is to have a client connect over SSL to a server wrapped with stunnel. Cheers.

@igorw
Copy link
Collaborator

igorw commented Apr 21, 2013

I've given this some thought, and UDP really does not fit with the current stream-oriented implementation of things. Thoughts? Can you rework/simplify the patch based on that?

@astephens25
Copy link
Author

Sorry it took me so long to get back to this. I agree on the removal of UDP. A missing packet from a connectionless stream would indeed be a problem for this application. I would be happy to simplify these modifications.

While using my modified version, I discovered an issue with the buffered reads when the read size I will go ahead and make the additional corrections for this as well.

Aaron Stephens added 3 commits May 15, 2013 00:59
…needed)

Modified argument parsing to allow passing array of parameters
Updated protocol tests for array argument and invalid argument type
@astephens25
Copy link
Author

All tests are passing. I made some modifications to the argument parsing in Protocol you may want to examine. I am using the code right now and everything seems to be working as intended.

I did not remove UDP per se, but I did remove any special code required to make it function. The documentation seems to indicate the normal socket client can open a UDP stream the same was as any other protocol, but there may be unforeseen issues with the "connection" lifecycle.

@igorw
Copy link
Collaborator

igorw commented Jul 8, 2013

Just figured out a deeper issue here. dnode-php is using React\Socket\Connection which is really made for server connections. A React\Stream\Stream (which uses fread) should be used instead. In fact, SocketClient provides one. So #26 would be a cleaner fix for this problem.

@rajesh41
Copy link

After forking, when run composer install or update followings error throws:-

  • A typo in the package name
  • The package is not available in a stable-enough version according to your minimum-stability setting

? to do PLease!!

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.

3 participants