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

Fixed incorrect proxy configuration of GitHubLoginFunction #109

Merged
merged 1 commit into from
Jan 28, 2016
Merged

Fixed incorrect proxy configuration of GitHubLoginFunction #109

merged 1 commit into from
Jan 28, 2016

Conversation

schulzh
Copy link
Contributor

@schulzh schulzh commented Jan 28, 2016

The Jenkins ProxyConfiguration class expects only the hostname (e.g. git.example.com vs https://git.example.com/api/) as a method parameter to createProxy, but the existing code gives it the whole URL.
As a result of this, the validation of no proxy hosts is not correct. In most cases this is not a problem, but it is for some enterprise GitHub instances.

The handling of the possible MalformedURLException might not be the most suitable, it may needs to be replaced.

Review on Reviewable

@KostyaSha
Copy link
Member

Wouldn't be better made matching in core?

@schulzh
Copy link
Contributor Author

schulzh commented Jan 28, 2016

I thought of changing this in the core aswell, but it explicitly says host as the method-parameter so strictly speaking the code in the plugin is wrong.

@KostyaSha
Copy link
Member

Then 👍 , let's wait for @lanwen review

@lanwen
Copy link
Member

lanwen commented Jan 28, 2016

Thanks for the fix!

lanwen added a commit that referenced this pull request Jan 28, 2016
Fixed incorrect proxy configuration of GitHubLoginFunction
@lanwen lanwen merged commit c43344a into jenkinsci:master Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants