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

Fix - Extend maven proxy settings usage to https #213

Closed
wants to merge 1 commit into from

Conversation

Eredrim
Copy link

@Eredrim Eredrim commented Apr 8, 2024

Why this pull request?

While I was reading the code of this project to find an elegant way to pass proxy settings to this maven plugin, I was pleasantly surprised to discover this plugin can read maven configuration to bind proxy settings. The test factory converts maven proxy configuration to java system properties but forgets these properties must be also set for https configurations. The goal of this pull request is to fix this, completing the missing implementation.

What does this code do?

This code keeps the current proxy detection and just adds the same settings for https. I don't make any distinction with https because maven proxy config uses one active proxy for all kind of requests, where java system properties makes a distinction between them.


Pull request requirements:

  • Please explain your motives to contribute this change: what problem you are trying to fix, what improvement you are trying to make
  • Use the following formatting style: SonarSource/sonar-developer-toolset
  • Provide a unit test for any code you changed
  • If there is a JIRA ticket available, please make your commits and pull request start with the ticket ID (MSONAR-XXXX).

@dorian-burihabwa-sonarsource
Copy link
Contributor

dorian-burihabwa-sonarsource commented May 28, 2024

Hi @Eredrim,

Thank you for resurfacing the topic and offering a solution. We worked on SCANMAVEN-183 to implement the support of https as a proxy protocol. We should roll out the new change in the next release of the scanner but feel free to try with a new build from the master branch to let us know if that works for you.

Thanks again for the suggestions.

@Eredrim
Copy link
Author

Eredrim commented Jun 7, 2024

Hello,

I tested the latest release version (https://github.com/SonarSource/sonar-scanner-maven/releases/tag/4.0.0.4121), it works like a charm! So, I close this PR.

@Eredrim Eredrim closed this Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants