Skip to content

Recognize HTTP URI schemes without regard to case - #3665

Merged
velo merged 6 commits into
OpenFeign:masterfrom
wozhenbang2004:fix/case-insensitive-http-schemes
Nov 9, 2026
Merged

velo merged 6 commits into
OpenFeign:masterfrom
wozhenbang2004:fix/case-insensitive-http-schemes

Conversation

@wozhenbang2004

Copy link
Copy Markdown
Contributor

A client targeting HTTP://host currently fails with target values must be absolute, even though the URI has an HTTP scheme. The same check affects HTTPS and mixed-case schemes. Fixing the template check alone still lets HardCodedTarget replace a per-request URI with the configured default target, while EmptyTarget continues to reject it.

Use a case-insensitive prefix check in UriUtils.isAbsolute, and share it with both target implementations. The scheme spelling is preserved in the request URL.

Regression coverage includes real requests to MockWebServer through a configured target, a per-request URI override, and an empty target, with lowercase, uppercase, and mixed-case HTTP schemes. Template tests also cover uppercase and mixed-case HTTP/HTTPS targets with paths, queries, and fragments, and ensure that absolute URLs are rejected by the relative-URI setter.

Validation:

  • New regressions fail on the original implementation and pass with the fix.
  • Core clean install with -Pdev -pl core -am, using JDK 8 for production sources and JDK 25 for tests: 783 tests, no failures or errors, 4 skips. Mockito was supplied as a premain Java agent for this environment.
  • License checks, code-format validation, module packaging, and binary compatibility checks pass.

@velo velo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Checked against master: isAbsolute, HardCodedTarget and EmptyTarget now share one case-insensitive check, and the tests fail on master and pass here. Nit, not blocking: like the old check, this is a prefix match, so a relative uri() literal that starts with 'HTTP' without a leading slash is now rejected as absolute, the same way a lowercase 'http' one already was. Matching 'http://' or 'https://' would be tighter, but that can come in a follow-up.

@velo
velo enabled auto-merge (squash) November 9, 2026 20:37
@velo

velo commented Nov 9, 2026

Copy link
Copy Markdown
Member

Approved, and auto-merge is enabled. It now conflicts with master, but only in CHANGELOG.md, because master already has a ### Version 13.18 section. Could you merge or rebase master and move your entry under that existing heading? (#3633 also adds to RequestTemplateTest, so if it lands first, you may need another quick rebase.)

Signed-off-by: Marvin Froeder <velo.br@gmail.com>

# Conflicts:
#	CHANGELOG.md
@velo

velo commented Nov 9, 2026

Copy link
Copy Markdown
Member

Merged master; CHANGELOG.md was removed from the repo so the entry was dropped.

@velo
velo merged commit c32cff5 into OpenFeign:master Nov 9, 2026
3 checks passed
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.

2 participants