You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Michael Osipov <mi...@apache.org> on 2019/12/28 12:40:07 UTC
Inconsistency/confusion on auth scheme names usage
Folks,
this has been bugging me for some time lately and I'd like to know your
opinion about it. I am also convinced that this may confuse some of your
client users:
We have:
* interface AuthScheme with String #getName()
* enum AuthSchemes which should be singular and maybe StandardAuthScheme
with the auth schemes we support out of the box. Or similar to
StandardCharsets have "final class StandardAuthSchemes" which has only
"final static Sting SYMBOLIC_NAME = "AuthSchemeName".
For me the enum implies that this is an exhaustive list of schemes we
support, nothing else more possible.
* class AuthScope with String #getAuthScheme(). Logically, I would
expect here to be returned an instance of AuthScheme and not a string.
Otherise it should be "String #getSchemeName()"
* HTTPCLIENT-2041, HTTPCLIENT-2042
* class AuthChallenge with "String #getScheme()" same with AuthScope
* class AuthExchange with "AuthSheme #getAuthScheme"
* RequestConfig "Collection<String
#(target|proxy)PreferredAuthSchemes()" same with AuthScope
and maybe other spots I have missed.
CookieSpec and friends have the same issue.
Am I just paranoid or do you consider them to be inconsistent as well?
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: Inconsistency/confusion on auth scheme names usage
Posted by Gary Gregory <ga...@gmail.com>.
On Sun, Dec 29, 2019 at 4:23 PM Michael Osipov <mi...@apache.org> wrote:
> Am 2019-12-29 um 21:08 schrieb Gary Gregory:
> > It just dawned on me that J. Bloch addresses enum extensibility by having
> > an enum implement an interface and having the API implemented in terms of
> > the interface.
> >
> > This gives you the best of both worlds: you get to define an enum with
> your
> > precanned values AND you allow users to provide custom implementations.
>
> I am aware of that scheme that this looks like a lot of complexity for
> something which does not need it.
>
> Does it?
>
From on high, no, but OTOH, I've not had the need to provide a custom auth
scheme so I cannot say how well such a thing would all look and feel in
code ;-)
But, we already have the 'complexity' by the mere fact that we have an auth
scheme interface AND enum!
So either we simplify what we have now or we clean it up, it just feels
weird to leave it as it.
The guts of the JRE has the enum sun.net.www.protocol.http.AuthScheme, no
interfaces, no classes, so who knows what the best design is...
Gary
>
> > On Sun, Dec 29, 2019, 08:15 Michael Osipov <mi...@apache.org> wrote:
> >
> >> Am 2019-12-28 um 17:17 schrieb Gary Gregory:
> >>> On Sat, Dec 28, 2019 at 7:40 AM Michael Osipov <mi...@apache.org>
> >> wrote:
> >>>
> >>>> Folks,
> >>>>
> >>>> this has been bugging me for some time lately and I'd like to know
> your
> >>>> opinion about it. I am also convinced that this may confuse some of
> your
> >>>> client users:
> >>>>
> >>>> We have:
> >>>> * interface AuthScheme with String #getName()
> >>>> * enum AuthSchemes which should be singular and maybe
> StandardAuthScheme
> >>>> with the auth schemes we support out of the box. Or similar to
> >>>> StandardCharsets have "final class StandardAuthSchemes" which has only
> >>>> "final static Sting SYMBOLIC_NAME = "AuthSchemeName".
> >>>> For me the enum implies that this is an exhaustive list of schemes we
> >>>> support, nothing else more possible.
> >>>>
> >>>
> >>> Thanks for bringing all of this up Michael :-)
> >>>
> >>> It feels like there is a disconnect indeed and that the code is not
> >>> complete or at least missing Javadoc:
> >>> - org.apache.hc.client5.http.impl.auth.SystemDefaultCredentialsProvider
> >>> uses hard coded strings instead of AuthSchemes id strings.
> >>> - All references to AuthSchemes use the AuthSchemes.id String instead
> of
> >> an
> >>> AuthSchemes itself (writing about this really shows the plural name as
> >>> lame.) Always using the id field seems to defeat the purpose of having
> an
> >>> enum in the first place.
> >>
> >> Exactly!
> >>
> >>> Do we intend for user pluggable AuthSchemes id String? It's confusing:
> >>> -
> >>
> org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy.DEFAULT_SCHEME_PRIORITY
> >>> is a List<String> instead of a List<AuthSchemes>
> >>> - This List is used with:
> >>> -
> >>>
> >>
> org.apache.hc.client5.http.config.RequestConfig.getProxyPreferredAuthSchemes()
> >>> returns List<String>
> >>> -
> >>>
> >>
> org.apache.hc.client5.http.config.RequestConfig.getTargetPreferredAuthSchemes()
> >>> returns List<String>
> >>>
> >>> That tells me I can provide whatever I want in RequestConfig in these
> two
> >>> scheme lists. Is that the intent or do these list must contain only
> valid
> >>> AuthSchemes ids? The Javadoc say nothing about the supported value
> range
> >>>
> (RequestConfig.Builder.setTargetPreferredAuthSchemes(Collection<String>))
> >>
> >> Yes, the usage of strings implies that you can put in whatever you want.
> >> This makes sense because of two reasons:
> >>
> >> * You can swap your default impls against your own
> >> * You can add you own auth schemes which operate over WWW-Authenticate,
> >> e.g., Bearer, HOBA, Mutual [1].
> >>
> >>> We ran into a similar-ish issue over at Log4j 2 with levels (INFO,
> WARN,
> >>> and so on) and I've had to deal with the same pattern elsewhere, so
> TL;DR
> >>> is if you want to define a type with known instances AND allow for
> custom
> >>> instances you cannot use an enum, you have to use a class and/or
> >> interface.
> >>> I am not sure of the intent here.
> >>
> >> Absolutely correct!
> >>
> >>> The fact that we have both an enum and an interface with effectively
> the
> >>> same name screams out for help ;-) java.nio.charset.StandardCharsets
> is a
> >>> good compromise and we could follow suit but let's address the above
> >> first
> >>> IMO.
> >>>
> >>> * class AuthScope with String #getAuthScheme(). Logically, I would
> >>>> expect here to be returned an instance of AuthScheme and not a string.
> >>>> Otherise it should be "String #getSchemeName()"
> >>>>
> >>>
> >>> Indeed, if we have an enum, let's use it, otherwise, do we need it at
> >> all?
> >>> If not we can go the java.nio.charset.StandardCharsets route. Maybe.
> >>>
> >>> Tangent:
> >>> - org.apache.hc.client5.http.auth.AuthSchemeProvider is Javadoc'd as a
> >>> factory class so why no call it AuthSchemeFactory? I think of providers
> >> as
> >>> larger scale components (like a security provider) where a factory is
> >>> smaller scale and creates instances. In this case, you cannot create
> >>> an AuthScheme since it is an enum, but no, wait, this factory is typed
> to
> >>> the interface so it does indeed create instances... we need to fix the
> >>> names of these types...
> >>
> >> I share this. For me a provider holds a collection a factory
> >> implementations for names entities.
> >>
> >> Please create a JIRA issue to track this.
> >>
> >> I have a few ideas, I will provide a PR soon for this.
> >>
> >> Michael
> >>
> >>> * HTTPCLIENT-2041, HTTPCLIENT-2042
> >>>> * class AuthChallenge with "String #getScheme()" same with AuthScope
> >>>> * class AuthExchange with "AuthSheme #getAuthScheme"
> >>>> * RequestConfig "Collection<String
> >>>> #(target|proxy)PreferredAuthSchemes()" same with AuthScope
> >>>>
> >>>> and maybe other spots I have missed.
> >>>>
> >>>> CookieSpec and friends have the same issue.
> >>>>
> >>>> Am I just paranoid or do you consider them to be inconsistent as well?
> >>>>
> >>>> Michael
> >>
> >> [1]
> >>
> >>
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication#Basic_authentication_scheme
> >>
> >
>
>
Re: Inconsistency/confusion on auth scheme names usage
Posted by Michael Osipov <mi...@apache.org>.
Am 2019-12-29 um 21:08 schrieb Gary Gregory:
> It just dawned on me that J. Bloch addresses enum extensibility by having
> an enum implement an interface and having the API implemented in terms of
> the interface.
>
> This gives you the best of both worlds: you get to define an enum with your
> precanned values AND you allow users to provide custom implementations.
I am aware of that scheme that this looks like a lot of complexity for
something which does not need it.
Does it?
> On Sun, Dec 29, 2019, 08:15 Michael Osipov <mi...@apache.org> wrote:
>
>> Am 2019-12-28 um 17:17 schrieb Gary Gregory:
>>> On Sat, Dec 28, 2019 at 7:40 AM Michael Osipov <mi...@apache.org>
>> wrote:
>>>
>>>> Folks,
>>>>
>>>> this has been bugging me for some time lately and I'd like to know your
>>>> opinion about it. I am also convinced that this may confuse some of your
>>>> client users:
>>>>
>>>> We have:
>>>> * interface AuthScheme with String #getName()
>>>> * enum AuthSchemes which should be singular and maybe StandardAuthScheme
>>>> with the auth schemes we support out of the box. Or similar to
>>>> StandardCharsets have "final class StandardAuthSchemes" which has only
>>>> "final static Sting SYMBOLIC_NAME = "AuthSchemeName".
>>>> For me the enum implies that this is an exhaustive list of schemes we
>>>> support, nothing else more possible.
>>>>
>>>
>>> Thanks for bringing all of this up Michael :-)
>>>
>>> It feels like there is a disconnect indeed and that the code is not
>>> complete or at least missing Javadoc:
>>> - org.apache.hc.client5.http.impl.auth.SystemDefaultCredentialsProvider
>>> uses hard coded strings instead of AuthSchemes id strings.
>>> - All references to AuthSchemes use the AuthSchemes.id String instead of
>> an
>>> AuthSchemes itself (writing about this really shows the plural name as
>>> lame.) Always using the id field seems to defeat the purpose of having an
>>> enum in the first place.
>>
>> Exactly!
>>
>>> Do we intend for user pluggable AuthSchemes id String? It's confusing:
>>> -
>> org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy.DEFAULT_SCHEME_PRIORITY
>>> is a List<String> instead of a List<AuthSchemes>
>>> - This List is used with:
>>> -
>>>
>> org.apache.hc.client5.http.config.RequestConfig.getProxyPreferredAuthSchemes()
>>> returns List<String>
>>> -
>>>
>> org.apache.hc.client5.http.config.RequestConfig.getTargetPreferredAuthSchemes()
>>> returns List<String>
>>>
>>> That tells me I can provide whatever I want in RequestConfig in these two
>>> scheme lists. Is that the intent or do these list must contain only valid
>>> AuthSchemes ids? The Javadoc say nothing about the supported value range
>>> (RequestConfig.Builder.setTargetPreferredAuthSchemes(Collection<String>))
>>
>> Yes, the usage of strings implies that you can put in whatever you want.
>> This makes sense because of two reasons:
>>
>> * You can swap your default impls against your own
>> * You can add you own auth schemes which operate over WWW-Authenticate,
>> e.g., Bearer, HOBA, Mutual [1].
>>
>>> We ran into a similar-ish issue over at Log4j 2 with levels (INFO, WARN,
>>> and so on) and I've had to deal with the same pattern elsewhere, so TL;DR
>>> is if you want to define a type with known instances AND allow for custom
>>> instances you cannot use an enum, you have to use a class and/or
>> interface.
>>> I am not sure of the intent here.
>>
>> Absolutely correct!
>>
>>> The fact that we have both an enum and an interface with effectively the
>>> same name screams out for help ;-) java.nio.charset.StandardCharsets is a
>>> good compromise and we could follow suit but let's address the above
>> first
>>> IMO.
>>>
>>> * class AuthScope with String #getAuthScheme(). Logically, I would
>>>> expect here to be returned an instance of AuthScheme and not a string.
>>>> Otherise it should be "String #getSchemeName()"
>>>>
>>>
>>> Indeed, if we have an enum, let's use it, otherwise, do we need it at
>> all?
>>> If not we can go the java.nio.charset.StandardCharsets route. Maybe.
>>>
>>> Tangent:
>>> - org.apache.hc.client5.http.auth.AuthSchemeProvider is Javadoc'd as a
>>> factory class so why no call it AuthSchemeFactory? I think of providers
>> as
>>> larger scale components (like a security provider) where a factory is
>>> smaller scale and creates instances. In this case, you cannot create
>>> an AuthScheme since it is an enum, but no, wait, this factory is typed to
>>> the interface so it does indeed create instances... we need to fix the
>>> names of these types...
>>
>> I share this. For me a provider holds a collection a factory
>> implementations for names entities.
>>
>> Please create a JIRA issue to track this.
>>
>> I have a few ideas, I will provide a PR soon for this.
>>
>> Michael
>>
>>> * HTTPCLIENT-2041, HTTPCLIENT-2042
>>>> * class AuthChallenge with "String #getScheme()" same with AuthScope
>>>> * class AuthExchange with "AuthSheme #getAuthScheme"
>>>> * RequestConfig "Collection<String
>>>> #(target|proxy)PreferredAuthSchemes()" same with AuthScope
>>>>
>>>> and maybe other spots I have missed.
>>>>
>>>> CookieSpec and friends have the same issue.
>>>>
>>>> Am I just paranoid or do you consider them to be inconsistent as well?
>>>>
>>>> Michael
>>
>> [1]
>>
>> https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication#Basic_authentication_scheme
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: Inconsistency/confusion on auth scheme names usage
Posted by Gary Gregory <ga...@gmail.com>.
It just dawned on me that J. Bloch addresses enum extensibility by having
an enum implement an interface and having the API implemented in terms of
the interface.
This gives you the best of both worlds: you get to define an enum with your
precanned values AND you allow users to provide custom implementations.
Gary
On Sun, Dec 29, 2019, 08:15 Michael Osipov <mi...@apache.org> wrote:
> Am 2019-12-28 um 17:17 schrieb Gary Gregory:
> > On Sat, Dec 28, 2019 at 7:40 AM Michael Osipov <mi...@apache.org>
> wrote:
> >
> >> Folks,
> >>
> >> this has been bugging me for some time lately and I'd like to know your
> >> opinion about it. I am also convinced that this may confuse some of your
> >> client users:
> >>
> >> We have:
> >> * interface AuthScheme with String #getName()
> >> * enum AuthSchemes which should be singular and maybe StandardAuthScheme
> >> with the auth schemes we support out of the box. Or similar to
> >> StandardCharsets have "final class StandardAuthSchemes" which has only
> >> "final static Sting SYMBOLIC_NAME = "AuthSchemeName".
> >> For me the enum implies that this is an exhaustive list of schemes we
> >> support, nothing else more possible.
> >>
> >
> > Thanks for bringing all of this up Michael :-)
> >
> > It feels like there is a disconnect indeed and that the code is not
> > complete or at least missing Javadoc:
> > - org.apache.hc.client5.http.impl.auth.SystemDefaultCredentialsProvider
> > uses hard coded strings instead of AuthSchemes id strings.
> > - All references to AuthSchemes use the AuthSchemes.id String instead of
> an
> > AuthSchemes itself (writing about this really shows the plural name as
> > lame.) Always using the id field seems to defeat the purpose of having an
> > enum in the first place.
>
> Exactly!
>
> > Do we intend for user pluggable AuthSchemes id String? It's confusing:
> > -
> org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy.DEFAULT_SCHEME_PRIORITY
> > is a List<String> instead of a List<AuthSchemes>
> > - This List is used with:
> > -
> >
> org.apache.hc.client5.http.config.RequestConfig.getProxyPreferredAuthSchemes()
> > returns List<String>
> > -
> >
> org.apache.hc.client5.http.config.RequestConfig.getTargetPreferredAuthSchemes()
> > returns List<String>
> >
> > That tells me I can provide whatever I want in RequestConfig in these two
> > scheme lists. Is that the intent or do these list must contain only valid
> > AuthSchemes ids? The Javadoc say nothing about the supported value range
> > (RequestConfig.Builder.setTargetPreferredAuthSchemes(Collection<String>))
>
> Yes, the usage of strings implies that you can put in whatever you want.
> This makes sense because of two reasons:
>
> * You can swap your default impls against your own
> * You can add you own auth schemes which operate over WWW-Authenticate,
> e.g., Bearer, HOBA, Mutual [1].
>
> > We ran into a similar-ish issue over at Log4j 2 with levels (INFO, WARN,
> > and so on) and I've had to deal with the same pattern elsewhere, so TL;DR
> > is if you want to define a type with known instances AND allow for custom
> > instances you cannot use an enum, you have to use a class and/or
> interface.
> > I am not sure of the intent here.
>
> Absolutely correct!
>
> > The fact that we have both an enum and an interface with effectively the
> > same name screams out for help ;-) java.nio.charset.StandardCharsets is a
> > good compromise and we could follow suit but let's address the above
> first
> > IMO.
> >
> > * class AuthScope with String #getAuthScheme(). Logically, I would
> >> expect here to be returned an instance of AuthScheme and not a string.
> >> Otherise it should be "String #getSchemeName()"
> >>
> >
> > Indeed, if we have an enum, let's use it, otherwise, do we need it at
> all?
> > If not we can go the java.nio.charset.StandardCharsets route. Maybe.
> >
> > Tangent:
> > - org.apache.hc.client5.http.auth.AuthSchemeProvider is Javadoc'd as a
> > factory class so why no call it AuthSchemeFactory? I think of providers
> as
> > larger scale components (like a security provider) where a factory is
> > smaller scale and creates instances. In this case, you cannot create
> > an AuthScheme since it is an enum, but no, wait, this factory is typed to
> > the interface so it does indeed create instances... we need to fix the
> > names of these types...
>
> I share this. For me a provider holds a collection a factory
> implementations for names entities.
>
> Please create a JIRA issue to track this.
>
> I have a few ideas, I will provide a PR soon for this.
>
> Michael
>
> > * HTTPCLIENT-2041, HTTPCLIENT-2042
> >> * class AuthChallenge with "String #getScheme()" same with AuthScope
> >> * class AuthExchange with "AuthSheme #getAuthScheme"
> >> * RequestConfig "Collection<String
> >> #(target|proxy)PreferredAuthSchemes()" same with AuthScope
> >>
> >> and maybe other spots I have missed.
> >>
> >> CookieSpec and friends have the same issue.
> >>
> >> Am I just paranoid or do you consider them to be inconsistent as well?
> >>
> >> Michael
>
> [1]
>
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication#Basic_authentication_scheme
>
Re: Inconsistency/confusion on auth scheme names usage
Posted by Michael Osipov <mi...@apache.org>.
Am 2019-12-28 um 17:17 schrieb Gary Gregory:
> On Sat, Dec 28, 2019 at 7:40 AM Michael Osipov <mi...@apache.org> wrote:
>
>> Folks,
>>
>> this has been bugging me for some time lately and I'd like to know your
>> opinion about it. I am also convinced that this may confuse some of your
>> client users:
>>
>> We have:
>> * interface AuthScheme with String #getName()
>> * enum AuthSchemes which should be singular and maybe StandardAuthScheme
>> with the auth schemes we support out of the box. Or similar to
>> StandardCharsets have "final class StandardAuthSchemes" which has only
>> "final static Sting SYMBOLIC_NAME = "AuthSchemeName".
>> For me the enum implies that this is an exhaustive list of schemes we
>> support, nothing else more possible.
>>
>
> Thanks for bringing all of this up Michael :-)
>
> It feels like there is a disconnect indeed and that the code is not
> complete or at least missing Javadoc:
> - org.apache.hc.client5.http.impl.auth.SystemDefaultCredentialsProvider
> uses hard coded strings instead of AuthSchemes id strings.
> - All references to AuthSchemes use the AuthSchemes.id String instead of an
> AuthSchemes itself (writing about this really shows the plural name as
> lame.) Always using the id field seems to defeat the purpose of having an
> enum in the first place.
Exactly!
> Do we intend for user pluggable AuthSchemes id String? It's confusing:
> - org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy.DEFAULT_SCHEME_PRIORITY
> is a List<String> instead of a List<AuthSchemes>
> - This List is used with:
> -
> org.apache.hc.client5.http.config.RequestConfig.getProxyPreferredAuthSchemes()
> returns List<String>
> -
> org.apache.hc.client5.http.config.RequestConfig.getTargetPreferredAuthSchemes()
> returns List<String>
>
> That tells me I can provide whatever I want in RequestConfig in these two
> scheme lists. Is that the intent or do these list must contain only valid
> AuthSchemes ids? The Javadoc say nothing about the supported value range
> (RequestConfig.Builder.setTargetPreferredAuthSchemes(Collection<String>))
Yes, the usage of strings implies that you can put in whatever you want.
This makes sense because of two reasons:
* You can swap your default impls against your own
* You can add you own auth schemes which operate over WWW-Authenticate,
e.g., Bearer, HOBA, Mutual [1].
> We ran into a similar-ish issue over at Log4j 2 with levels (INFO, WARN,
> and so on) and I've had to deal with the same pattern elsewhere, so TL;DR
> is if you want to define a type with known instances AND allow for custom
> instances you cannot use an enum, you have to use a class and/or interface.
> I am not sure of the intent here.
Absolutely correct!
> The fact that we have both an enum and an interface with effectively the
> same name screams out for help ;-) java.nio.charset.StandardCharsets is a
> good compromise and we could follow suit but let's address the above first
> IMO.
>
> * class AuthScope with String #getAuthScheme(). Logically, I would
>> expect here to be returned an instance of AuthScheme and not a string.
>> Otherise it should be "String #getSchemeName()"
>>
>
> Indeed, if we have an enum, let's use it, otherwise, do we need it at all?
> If not we can go the java.nio.charset.StandardCharsets route. Maybe.
>
> Tangent:
> - org.apache.hc.client5.http.auth.AuthSchemeProvider is Javadoc'd as a
> factory class so why no call it AuthSchemeFactory? I think of providers as
> larger scale components (like a security provider) where a factory is
> smaller scale and creates instances. In this case, you cannot create
> an AuthScheme since it is an enum, but no, wait, this factory is typed to
> the interface so it does indeed create instances... we need to fix the
> names of these types...
I share this. For me a provider holds a collection a factory
implementations for names entities.
Please create a JIRA issue to track this.
I have a few ideas, I will provide a PR soon for this.
Michael
> * HTTPCLIENT-2041, HTTPCLIENT-2042
>> * class AuthChallenge with "String #getScheme()" same with AuthScope
>> * class AuthExchange with "AuthSheme #getAuthScheme"
>> * RequestConfig "Collection<String
>> #(target|proxy)PreferredAuthSchemes()" same with AuthScope
>>
>> and maybe other spots I have missed.
>>
>> CookieSpec and friends have the same issue.
>>
>> Am I just paranoid or do you consider them to be inconsistent as well?
>>
>> Michael
[1]
https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication#Basic_authentication_scheme
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: Inconsistency/confusion on auth scheme names usage
Posted by Gary Gregory <ga...@gmail.com>.
On Sat, Dec 28, 2019 at 7:40 AM Michael Osipov <mi...@apache.org> wrote:
> Folks,
>
> this has been bugging me for some time lately and I'd like to know your
> opinion about it. I am also convinced that this may confuse some of your
> client users:
>
> We have:
> * interface AuthScheme with String #getName()
> * enum AuthSchemes which should be singular and maybe StandardAuthScheme
> with the auth schemes we support out of the box. Or similar to
> StandardCharsets have "final class StandardAuthSchemes" which has only
> "final static Sting SYMBOLIC_NAME = "AuthSchemeName".
> For me the enum implies that this is an exhaustive list of schemes we
> support, nothing else more possible.
>
Thanks for bringing all of this up Michael :-)
It feels like there is a disconnect indeed and that the code is not
complete or at least missing Javadoc:
- org.apache.hc.client5.http.impl.auth.SystemDefaultCredentialsProvider
uses hard coded strings instead of AuthSchemes id strings.
- All references to AuthSchemes use the AuthSchemes.id String instead of an
AuthSchemes itself (writing about this really shows the plural name as
lame.) Always using the id field seems to defeat the purpose of having an
enum in the first place.
Do we intend for user pluggable AuthSchemes id String? It's confusing:
- org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy.DEFAULT_SCHEME_PRIORITY
is a List<String> instead of a List<AuthSchemes>
- This List is used with:
-
org.apache.hc.client5.http.config.RequestConfig.getProxyPreferredAuthSchemes()
returns List<String>
-
org.apache.hc.client5.http.config.RequestConfig.getTargetPreferredAuthSchemes()
returns List<String>
That tells me I can provide whatever I want in RequestConfig in these two
scheme lists. Is that the intent or do these list must contain only valid
AuthSchemes ids? The Javadoc say nothing about the supported value range
(RequestConfig.Builder.setTargetPreferredAuthSchemes(Collection<String>))
We ran into a similar-ish issue over at Log4j 2 with levels (INFO, WARN,
and so on) and I've had to deal with the same pattern elsewhere, so TL;DR
is if you want to define a type with known instances AND allow for custom
instances you cannot use an enum, you have to use a class and/or interface.
I am not sure of the intent here.
The fact that we have both an enum and an interface with effectively the
same name screams out for help ;-) java.nio.charset.StandardCharsets is a
good compromise and we could follow suit but let's address the above first
IMO.
* class AuthScope with String #getAuthScheme(). Logically, I would
> expect here to be returned an instance of AuthScheme and not a string.
> Otherise it should be "String #getSchemeName()"
>
Indeed, if we have an enum, let's use it, otherwise, do we need it at all?
If not we can go the java.nio.charset.StandardCharsets route. Maybe.
Tangent:
- org.apache.hc.client5.http.auth.AuthSchemeProvider is Javadoc'd as a
factory class so why no call it AuthSchemeFactory? I think of providers as
larger scale components (like a security provider) where a factory is
smaller scale and creates instances. In this case, you cannot create
an AuthScheme since it is an enum, but no, wait, this factory is typed to
the interface so it does indeed create instances... we need to fix the
names of these types...
Gary
* HTTPCLIENT-2041, HTTPCLIENT-2042
> * class AuthChallenge with "String #getScheme()" same with AuthScope
> * class AuthExchange with "AuthSheme #getAuthScheme"
> * RequestConfig "Collection<String
> #(target|proxy)PreferredAuthSchemes()" same with AuthScope
>
> and maybe other spots I have missed.
>
> CookieSpec and friends have the same issue.
>
> Am I just paranoid or do you consider them to be inconsistent as well?
>
> Michael
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>