Skip to content

Conversation

@mkarg
Copy link
Contributor

@mkarg mkarg commented Oct 3, 2022

In an earlier discussion @jansupol and me proposed to allow specifying the separation character for getHeaderString, as some header values actually may contain commas, so being able to separate them later bears the need to have non-comma separation.

@mkarg mkarg added this to the 4.0 milestone Oct 3, 2022
@mkarg mkarg requested a review from jansupol October 3, 2022 08:31
@mkarg mkarg self-assigned this Oct 3, 2022
Copy link
Contributor

@chkal chkal left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@mkarg
Copy link
Contributor Author

mkarg commented Oct 3, 2022

@jansupol Kindly requesting your review, as this idea originated from a discussion with you. :-)

@mkarg
Copy link
Contributor Author

mkarg commented Nov 1, 2022

@jansupol WDYT?

@jansupol
Copy link
Contributor

jansupol commented Nov 3, 2022

Thinking about it, does it not clash a bit with a HeaderDelegate, which is used to convert the Object to String (and String to Object). So the methods on ClientRequest and ContainerReponse should use the HeaderDelegate.

On one hand, the HeaderDelegate can be enriched with the separator, but on the other hand, the Header delegate should be aware of the proper separator type. So the separator on these outgoing sides (and partially on HttpHeaders for the outgoing side, like filters) looks like it asks the implementation to reparse the HeaderDelegate result with the new separator, but the original separator is not known (the HeaderDelegate knows it).

@mkarg
Copy link
Contributor Author

mkarg commented Dec 29, 2022

@jansupol I do not think that this has anything to do with HeaderDelegate (or I misunderstood your comment). The reason is that HeaderDelegate is dealing with one single header value, so it is invoked multiple times when receiving a list of values (like X-My-Header val1;val2;val3;val4;val5). Which separation character (here: semicolon) is wanted is completely decided by the application, because the header could be a custom one (here: X-My-Header), so HeaderDelegate simply cannot know which separation character is to be used. This PR just replaces the hard-coded "always comma" limitation by a "application can decide" freedom, which has not the least to do with HeaderDelegate.

@mkarg mkarg marked this pull request as ready for review January 22, 2023 18:17
@spericas
Copy link
Contributor

@jansupol @mkarg Any thoughts about this PR? Keep it or close it?

@jansupol
Copy link
Contributor

Jersey HeaderDelegates are application-specific, as we have SPI to register user-specific delegates, leading to @mkarg 's "application can decide" freedom. The Spec itself does not have the ability.

Maybe the other frameworks do have this ability, too and it could be added to the spec?

@mkarg
Copy link
Contributor Author

mkarg commented Dec 12, 2023

keep

@spericas spericas modified the milestones: 4.0, 5.0 Mar 25, 2024
@mkarg
Copy link
Contributor Author

mkarg commented Dec 17, 2024

Kindly asking to reconsider this feature. Thanks.

@mkarg
Copy link
Contributor Author

mkarg commented Mar 30, 2025

@jamezp WDYT?

Copy link
Contributor

@jamezp jamezp left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me.

@mkarg mkarg merged commit 36de360 into jakartaee:main Apr 2, 2025
@mkarg mkarg deleted the getheaderstring-separator branch April 2, 2025 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

5 participants