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

JsonRPCClient constructor is missing the $proxy parameter #6

Closed
cavo789 opened this issue Sep 17, 2019 · 4 comments
Closed

JsonRPCClient constructor is missing the $proxy parameter #6

cavo789 opened this issue Sep 17, 2019 · 4 comments

Comments

@cavo789
Copy link

cavo789 commented Sep 17, 2019

Hello

I've notice that you're using a $proxy parameter in your constructor but that parameter isn't defined in your function:

https://github.com/weberhofer/jsonrpcphp/blob/master/src/org/jsonrpcphp/JsonRPCClient.php#L72-L88

The correct code, I think, has to be:

     /**
     * Takes the connection parameters.
     *
     * @param string $url
     * @param bool   $debug
     * @param mixed  $proxy
     */
    public function __construct($url, $debug = false, $proxy = '')
    {
        // server URL
        $this->url = $url;
        // proxy
        empty($proxy) ? $this->proxy = '' : $this->proxy = $proxy;
		// debug state
        empty($debug) ? $this->debug = false : $this->debug = true;
        // message id
        $this->id = 1;
    }

I've tried to create a new branch and to push a Pull request but I don't have the permission to do this. Perhaps something to change in the settings of your current repo.

Thanks!

@cavo789
Copy link
Author

cavo789 commented Nov 19, 2019

Forgot to also mention that the __call function should also be updated to add these lines:

if ('' !== trim($this->proxy)) {
    curl_setopt($ch, CURLOPT_PROXY, $this->proxy);
}

@weberhofer
Copy link
Owner

Thanks @cavo789 for notifying me. I have fixed the code. Btw: Before you do changes you like to submit you have to fork the project.

@cavo789
Copy link
Author

cavo789 commented Dec 3, 2019

Hello.

Thanks for the update.

For the fork, I had tried to create a fork but unless I was mistaken; this was not possible at that time (maybe you had forbidden the creation of a fork and the submission of a PR?)

Have a nice evening.

@weberhofer
Copy link
Owner

@cavo789, there is nothing special on that repo. You definitely should be able to fork except there is/was something wrong at github.

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