Skip to content

fix: Configuration to be able to control sending RFC 9457 related Accept HTTP header - #1891

Merged
cstamas merged 3 commits into
apache:masterfrom
cstamas:rfc-9457-control
May 27, 2026
Merged

cstamas merged 3 commits into
apache:masterfrom
cstamas:rfc-9457-control

Conversation

@cstamas

@cstamas cstamas commented May 27, 2026

Copy link
Copy Markdown
Member

Some servers have issues handling it (esp the Accept HTTP header), so make it possible to disable it. Moreover, the new config accepts REPO_ID suffix, so this can happen per-server basis.

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Your pull request should address just one issue, without pulling in other changes.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body.
    Note that commits might be squashed by a maintainer on merge.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
    This may not always be possible but is a best-practice.
  • Run mvn verify to make sure basic checks pass.
    A more thorough check will be performed on your pull request automatically.
  • You have run the integration tests successfully (mvn -Prun-its verify).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

Some servers have issues handling it (esp the Accept HTTP header),
so make it possible to disable it. Moreover, the new config
accepts REPO_ID suffix, so this can happen per-server basis.
@cstamas cstamas added this to the 2.0.19 milestone May 27, 2026
@cstamas
cstamas requested review from kwin and slawekjaranowski May 27, 2026 10:15
@cstamas cstamas self-assigned this May 27, 2026
@cstamas cstamas added the enhancement New feature or request label May 27, 2026
@cstamas
cstamas marked this pull request as ready for review May 27, 2026 10:36

@kwin kwin 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.

In addition we need to add Accept for regular content (either wildcard for all requests or xml/binary depending on whether it is metadata or artifacts) when we add the accept for problem+json

* @configurationDefaultValue {@link #DEFAULT_HTTP_SUPPORT_RFC9457}
* @configurationRepoIdSuffix Yes
*/
public static final String HTTP_SUPPORT_RFC9457 = PREFIX_TRANSPORT_HTTP + "supportRfc9457";

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.

This is a confusing comment to me, as this is just about the accept header which is not actually specified in the RFC. I would therefore rather refer to it as EXPLICITLY_ACCEPT_PROBLEM_JSON

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This and @slawekjaranowski comment below makes sense, this config should be ONLY about Accept header, and then as Slawek says below, just always parse response?

Comment on lines +533 to +536
if (supportRfc9457) {
ApacheRFC9457Reporter.INSTANCE.generateException(response, (statusCode, reasonPhrase) -> {
throw new HttpResponseException(statusCode, reasonPhrase + " (" + statusCode + ")");
});

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.

maybe we can always parse response?

@cstamas

cstamas commented May 27, 2026

Copy link
Copy Markdown
Member Author

In addition we need to add Accept for regular content (either wildcard for all requests or xml/binary depending on whether it is metadata or artifacts) when we add the accept for problem+json

I find this problematic: adding xml or json or any other Accept is not right; transport is only about artifacts, and as we know, servers knows to be picky. So, ideally I'd just modific the text of this config param (it is ONLY about adding Accept to fully support RFC 9457), because historically Resolver never sent any Accept header at all.

@cstamas

cstamas commented May 27, 2026

Copy link
Copy Markdown
Member Author

Also, in this very case, it just happens that we use a REST endpoint of an older software. Usually, Resolver transport is hitting non REST.

And this happens ONLY as nx2 offered "transparent support" for maven-deploy-plugin, while deployment to staging was always possible via alternate means (native plugin or so).

@kwin kwin 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.

I would still add */* to accept header values (in case they are sent) but can be done in a separate PR.

@cstamas cstamas changed the title fix: Configuration to be able to control whether RFC 9457 is supported fix: Configuration to be able to control sending RFC 9457 related Accept HTTP header May 27, 2026
@cstamas
cstamas merged commit a44bed7 into apache:master May 27, 2026
14 checks passed
@cstamas
cstamas deleted the rfc-9457-control branch May 27, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants