I have done an initial commit but there are no changes yet w.r.t.
modifications of the API as we have discussed.
I look the liberty of creating a separate module in the contribs
section and rearranged some of the package/class names.
I also wanted to explore something like this:
Client c = ApacheHttpClient.create();
so that one does not have to refer to directly to the handler and the
pattern is similar.
Paul.
On Nov 21, 2008, at 3:18 PM, Jorge L. Williams wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>> On Nov 20, 2008, at 6:30 PM, Jorge L Williams wrote:
>>
>>>
>>>
>>> Paul.Sandoz_at_Sun.COM wrote on 11/20/2008 08:08:05 AM:
>>>
>>>>
>>>> On Nov 20, 2008, at 3:42 PM, Jorge L. Williams wrote:
>>>>
>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>> Hash: SHA1
>>>>>
>>>>> Paul Sandoz wrote:
>>>>>> Hi Jorge,
>>>>>>
>>>>>> I have looked more closely at the patch, some comments and
>>> proposed
>>>>>> improvements:
>>>>>>
>>>>>> - Alex pointed out to me that by default interactive prompting
>>> for
>>>>>> credentials occurs. I think the default value for
>>>>>> PROPERTY_INTERACTIVE and PROPERTY_PREEMPTIVE_AUTHENTICATION
>>> should
>>>>>> be false, since by client API is likely to be used in
>>>>>> non-interactive environments.
>>>>>>
>>>>>
>>>>> The default for PROPERTY_PREEMPTIVE_AUTHENTICATION is already
>>> false.
>>>>>
>>>>> As for, PROPERTY_INTERACTIVE, I distinctly remember that
>>> prompting
>>>>> for credentials was the default behavior when using
>>> URLConnection and
>>>>> I wanted to match the behavior in case people were relying on
>>> it. Now
>>>>> that I look at the documentation, I guess I must have dreamt that
>>>>> behavior because I can't find a reference to it. Keep in mind
>>> that,
>>>>> if you set interactive to true, but also set the credentials in
>>> the
>>>>> Config you will *not* be prompt.
>>>>
>>>> OK.
>>>>
>>>>
>>>
>>> Just to clarify: Does this mean you're okay with the current default
>>> of setting interactive to true??
>>>
>>
>> No. It means that i understood what you said.
>>
>>
>
> Okay, gotcha.
>>>
>>>>> None the less, I really don't have
>>>>> a problem with setting the default to false.
>>>>>
>>>>>> - Rename: PROPERTY_INTERACTIVE ->
>>>>>> PROPERTY_AUTHENTICATION_INTERACTIVE
>>>>>> PROPERTY_PREEMPTIVE_AUTHENTICATION ->
>>>>>> PROPERTY_AUTHENTICATION_PREEMPTIVE
>>>>>>
>>>>> Good idea, no problem.
>>>>>
>>>>>> - Change the properties PROPERTY_PROXY_SET, PROPERTY_PROXY_HOST,
>>>>>> PROPERTY_PROXY_PORT to one property PROPERTY_PROXY that is a
>>> string
>>>>>> or URI, which if the port is absent then 8080 is used.
>>>>>>
>>>>> Another good idea. Here, again, I was trying to match the
>>> behavior
>>>>> of the java.net package.
>>>>>> - Have a method HttpClientConfig.getHttpState() thus the
>>>>>> credential-based methods could be re-factored to a helper class
>>> or
>>>>>> just reside on the DefaultHttpClientConfig.
>>>>>>
>>>>> Okay, here I disagree. I really don't like the developer having
>>> to
>>>>> know anything about HttpClient in the typical case. Not
>>> having to
>>>>> visit the Javadocs for the HttpClient api just makes the backend
>>>>> easier to use. I think setting credentials will occur quite
>>> frequently
>>>>> so I'm a little reluctant to have the user have to manage an
>>> HttpState
>>>>> or have to know anything about it to work with the ClientHandler.
>>>>>
>>>>> Of course, I'm open to the idea, but I guess I don't understand
>>> why
>>>>> you're making this suggestion(?) Is it because you feel it will
>>>>> address a security issue? Please explain.
>>>>
>>>> I am guessing that there may be other ways to configure the
>>> HttpState
>>>> beyond the common cases.
>>>>
>>>> A way to get or set the HttpState might be useful. A filter could
>>>> create/use a different HttpState and set it on the properties
>>>> e.g. i
>>>> could have credentials set up for a particular WebResource using a
>>>> filter to add the HttpState to the properties.
>>>>
>>>> I was not proposing to remove the methods or for the ClientHandler
>>> to
>>>> be utilized. I think they are useful, and maybe so in a different
>>>> context.
>>>>
>>>>
>>>
>>> Okay. So long as we keep the simple credential setting methods on
>>> HttpClientConfig, then I don't have a problem exposing the HttpState
>>> through a property or by adding a getHttpState() method. Keep in
>>> mind that getHttpState() and the property PROPERTY_HTTP_STATE
>>> already exists in DefaultHttpClientConfig -- we'll just need to move
>>> these over.
>>>
>>
>> What about instead if we have a class HttpClientState that
>> encapsulates HttpState and has those helper methods on it. Then we
>> can
>> expose a property for HttpClientState and getters/setters. That way
>> it
>> is more resuable and with a bit of JavaDoc we can easily reference
>> how
>> to set credentials, for example:
>>
>> HttpClientConfig.getState().setCredentials(...);
>>
>>
>>
>
>
> That sounds reasonable. I'm okay with that.
>
>>>
>>>>>
>>>>>> - I think we need to implement a RequestEntity specific to
>>> writing
>>>>>> so that we do not buffer the request.
>>>>>>
>>>>> We are in accord on this.
>>>>>
>>>>> I'm spread a bit thin these days and won't be able to address
>>> these
>>>>> issues until sometime next week.
>>>>>
>>>>
>>>>
>>>> I will try and do something tomorrow,
>>>> Paul.
>>>>
>>>
>>> Thank you. Keep me posted, and if there's a chance I'll contribute
>>> some next week.
>>>
>>
>> Some unit tests would be most helpful.
> Can't make promises, but I'll see if I can fit that in.
>
>
> - -jOrGe W.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.9 (GNU/Linux)
>
> iEYEARECAAYFAkkmwx0ACgkQ72r0i/n44hOmTgCgnmuxKlQEbNSp744L2Rv67DTH
> dPwAn2N1bmWYbGr18o6RVNcMZaVaJuCI
> =ovQ0
> -----END PGP SIGNATURE-----
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>