search

jakartaee / rest

Jakarta RESTful Web Services
Other
361 stars 117 forks source link

Can specify separation character for getHeaderString #1134

Open mkarg opened 2 years ago

mkarg commented 2 years ago

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 commented 2 years ago

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

mkarg commented 1 year ago

@jansupol WDYT?

jansupol commented 1 year ago

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 commented 1 year ago

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

spericas commented 9 months ago

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

jansupol commented 9 months ago

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 commented 9 months ago

keep