-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Correctly handle emtpy string in proxyuserpwd config #16644
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cleaner IMO. since it sets the correct default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome :)
server/tests/lib/Http/Client/ClientTest.php Lines 56 to 65 in effca30
@rullzer do you know why these tests are not failing? |
I think because null is not matched exactly in the with statement... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty
might also have done the trick but this is fine :)
Thanks a lot!
one could then try $this->config
->expects($this->at(1))
->method('getSystemValue')
->with($this->equalTo('proxyuserpwd'), $this->equalTo(null))
->willReturn(null); That might help in case phpunit filters out empty args. |
Ah my bad. pushed wrong. let me create a proper branch. |
As documented, the default value for config value proxyuserpwd is ''.
However, that value results in the error:
"cURL error 5: Unsupported proxy syntax in '@'".
This patch handles the values of '' and null (the default in the code)
the same for config values proxyuserpwd and proxy.
Signed-off-by: Scott Shambarger [email protected]