Skip to content

fix #3855: (http-support): correct native request builder URI/body/header sem…#3856

Open
YianZhao wants to merge 3 commits intoeclipse-sw360:mainfrom
YianZhao:bug-alt-request-builder-semantics
Open

fix #3855: (http-support): correct native request builder URI/body/header sem…#3856
YianZhao wants to merge 3 commits intoeclipse-sw360:mainfrom
YianZhao:bug-alt-request-builder-semantics

Conversation

@YianZhao
Copy link
Contributor

Summary

This PR fixes request-construction semantics in the native HTTP client path (newHttpClientAlt) by updating NewRequestBuilderImpl.

What was wrong

NewRequestBuilderImpl had three behavior issues:

  1. Invalid URIs were silently ignored.
  2. Requests without an explicit body could be built through a null-body path.
  3. Content-Type: application/json was forced for all requests in build().

These behaviors are inconsistent with expected HTTP request semantics and can produce incorrect requests.

Changes

Code changes

  • Updated uri(String uri) to use URI.create(uri) directly, so invalid URIs fail fast with IllegalArgumentException.
  • Updated build() to use BodyPublishers.noBody() when no body is provided.
  • Removed unconditional Content-Type: application/json injection from build().

Tests added

Added new unit test class:

  • NewRequestBuilderImplTest

Covered scenarios:

  • invalid URI is rejected,
  • GET without body builds successfully and does not set forced content type,
  • POST without body builds successfully and does not set forced content type,
  • explicitly set Content-Type is preserved and not overridden.

Verification

Executed:

mvn -pl clients/http-support "-Dbase.deploy.dir=." "-DskipTests=false" "-Dmaven.test.skip=false" "-Dtest=NewRequestBuilderImplTest" test

@GMishx GMishx added needs code review needs general test This is general testing, meaning that there is no org specific issue to check for labels Mar 12, 2026
@YianZhao
Copy link
Contributor Author

Thanks for taking a look. All checks are passing on my side now. Please let me know if any changes are needed when you review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs code review needs general test This is general testing, meaning that there is no org specific issue to check for

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants