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-11364 Proxy Controller and Monkifier #11365

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

Conversation

JREastonMarks
Copy link
Contributor

@JREastonMarks JREastonMarks commented Jan 30, 2025

Fix #11364
Updates the proxy controller to be able to handle multiple proxy accounts in a secure manner. Updates the Monkifier to be a static utility class instead of a component.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
3 Security Hotspots
C Security Rating on New Code (required ≥ A)
4 New Vulnerabilities (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@JREastonMarks
Copy link
Contributor Author

JREastonMarks commented Jan 30, 2025

These 2 security issues and 4 new vulnerabilities are from legacy code that was renamed in this project. I am not sure of how much those methods are used.

The third security issue is an introduced one but sonarcube does not like all HTTP requests to go into a single method. However, this is what is required of the proxy.

@JREastonMarks JREastonMarks marked this pull request as ready for review January 30, 2025 21:21
@inodb inodb requested a review from onursumer February 3, 2025 17:21
Copy link
Member

@onursumer onursumer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general. Just requested a few minor changes.

Comment on lines +55 to +68
String service = proxy.path("/proxy/").toString();

int endPosition = service.indexOf("/");
if (endPosition >= 0) {
service = service.substring(0, endPosition);
}

if (!routes.containsKey(service)) {
throw new UnknownServiceException();
}

String updatedPath = proxy.path("/proxy/" + service).trim();

String proxyServerHost = routes.get(service).trim();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to extract this logic to a separate method, something like getValidatedService

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the indentation in this file is using tabs instead of spaces. I suspect deleting ProxyController and adding a brand new file caused this issue. I think just renaming the file and modifying the contents would be better. That way we can still keep the change history.

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

Successfully merging this pull request may close these issues.

Proxy Controller Updates
2 participants