[TT-5790] Update EnsureTransport and related tests

[titpetric]

User description

Several tests encountered parsing errors where the URL parser complained that "the first path segment in URL cannot contain colon". This occurred because the URLs were missing the scheme (e.g., http://) and couldn't be parsed.

This PR improves EnsureTransport to better construct a valid URL from the host and protocol parameters.

• Improves test with more test cases for IP:port and protocol pairs
• Verifies intended behaviour with h2c URL parsing (coalescing to http scheme) with a unit test

https://tyktech.atlassian.net/browse/TT-5790

---

Type

enhancement, tests

---

Description

• Refactored EnsureTransport to automatically handle missing protocols by defaulting to "http" and to prepend the protocol if it's missing in the host string.
• Removed outdated switch-case logic in EnsureTransport, simplifying the URL scheme handling.
• Expanded unit tests in reverse_proxy_test.go to cover new scenarios including IP addresses with ports, and protocol handling.
• Improved test assertions to use assert.Equal and added checks to ensure URLs are parsable after processing.

---

Changes walkthrough

	Relevant files
Enhancement	reverse_proxy.go Refactor EnsureTransport to handle URL schemes more robustly gateway/reverse_proxy.go Added logic to EnsureTransport to handle missing protocols by defaulting to "http". Amended the function to prepend protocol if missing in the host string. Replaced specific scheme handling with a more generic approach. Removed old switch-case logic that handled schemes. +14/-9
Tests	reverse_proxy_test.go Extend tests for EnsureTransport with more scenarios          gateway/reverse_proxy_test.go Expanded test cases for EnsureTransport to cover scenarios with IP addresses and ports. Added tests for default protocol handling and h2c to http conversion. Replaced error checks with assert.Equal and added URL parse verification. +22/-3

---

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

API Changes

--- prev.txt	2024-05-14 11:46:26.886532924 +0000
+++ current.txt	2024-05-14 11:46:23.658522169 +0000
@@ -7199,6 +7199,8 @@
     it to base64 and store it in an Event object
 
 func EnsureTransport(host, protocol string) string
+    EnsureTransport sanitizes host/protocol pairs and returns a valid URL.
+
 func GenerateTestBinaryData() (buf *bytes.Buffer)
 func GetAccessDefinitionByAPIIDOrSession(currentSession *user.SessionState, api *APISpec) (accessDef *user.AccessDefinition, allowanceScope string, err error)
 func GetTLSClient(cert *tls.Certificate, caCert []byte) *http.Client

[github-actions]

PR Description updated to latest commit (5478721)

[github-actions]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves both logic changes in URL handling and extensive updates to unit tests. Understanding the implications of these changes on existing functionalities and ensuring no regressions occur would require a moderate level of effort.
🧪 Relevant tests	Yes
🔍 Possible issues	Possible Bug: The replacement of "h2c://" with "http://" in EnsureTransport function might not handle cases where other parts of the codebase explicitly rely on "h2c" as a distinct protocol.
🔒 Security concerns	No

Code feedback:

---

relevant file	gateway/reverse_proxy.go
suggestion	Consider handling the case where url.Parse fails more gracefully. Currently, if url.Parse fails, the function returns the original host string, which might not be a valid URL. This could lead to further errors down the line. Instead, consider logging the error or even returning an error from the function to handle it upstream. [important]
relevant line	if err != nil {

---

relevant file	gateway/reverse_proxy.go
suggestion	Add a normalization step for the protocol to handle different cases (e.g., "HTTP", "http") uniformly. This can prevent issues with case-sensitive protocol handling elsewhere in the code or when interfacing with other systems that might expect lowercase protocols. [medium]
relevant line	protocol = strings.TrimSpace(protocol)

---

relevant file	gateway/reverse_proxy.go
suggestion	Consider verifying the scheme after appending it to host. This can ensure that the resulting URL is valid and the scheme is correctly formatted before attempting to parse it with url.Parse. This could prevent subtle bugs related to malformed URLs. [important]
relevant line	host = protocol + "://" + host

---

relevant file	gateway/reverse_proxy_test.go
suggestion	Expand the test cases to include more edge cases, such as hosts with unusual characters or international domain names (IDN). This will ensure that the EnsureTransport function robustly handles a wider variety of inputs. [medium]
relevant line	{"httpbin.org:2000 ", "", "http://httpbin.org:2000"},

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[github-actions]

PR Code Suggestions

Category	Suggestions
Enhancement	Improve the robustness of protocol replacement in URLs. Replace the string replacement for 'h2c' protocol with a more robust approach using URL parsing and scheme modification. This ensures that only the scheme part of the URL is modified without affecting other parts of the URL that might accidentally contain the substring 'h2c://'. gateway/reverse_proxy.go [142] -host = strings.Replace(host, "h2c://", "http://", 1) +u, _ := url.Parse(host) +if u.Scheme == "h2c" { + u.Scheme = "http" + host = u.String() +}
	Enhance test coverage for EnsureTransport with more complex URL scenarios. Add more test cases to cover scenarios where the input host includes unusual characters or patterns to ensure EnsureTransport handles these gracefully. gateway/reverse_proxy_test.go [1599] {"httpbin.org:2000 ", "", "http://httpbin.org:2000"}, +{"http://example.com/with_path?query=value", "https", "https://example.com/with_path?query=value"}, +{"user@httpbin.org", "https", "https://user@httpbin.org"},
Error handling	Add error handling for URL parsing failures. Add error handling for URL parsing to prevent returning an invalid URL when an error occurs. This ensures that the function behaves predictably in error scenarios. gateway/reverse_proxy.go [144-146] u, err := url.Parse(host) if err != nil { - return host + return "", err }
Maintainability	Refactor protocol handling to use a switch statement for better extensibility. Use a switch statement to handle different protocols more cleanly and extendibly, instead of using an if condition. This makes it easier to add new protocols in the future. gateway/reverse_proxy.go [133-135] -if protocol == "" { +switch protocol { +case "": protocol = "http" +case "h2c": + protocol = "http" +default: + // no change }
	Improve test case identification in EnsureTransport tests. Replace the manual test case naming with a more descriptive approach using the host and protocol for easier identification of failing cases. gateway/reverse_proxy_test.go [1612] -t.Run(fmt.Sprintf("case-%d", i), func(t *testing.T) { +t.Run(fmt.Sprintf("host-%s_protocol-%s", v.host, v.protocol), func(t *testing.T) {

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[github-actions]

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[titpetric]

/release to release-5.3

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

@titpetric Succesfully merged PR