Parsing of URL's which contain Windows Path separator

[Shirish Kuncolienkar]

Hi,

I have the following piece of code

URL url = new URL("file", "/", "C:\\temp\\Java6");
System.out.println(url);
URL url1 = new URL(url, "hello.html");
System.out.println(url1);

first System out prints as "file:///C:\temp\Java6\Lotus"
Second one prints the value "file:hello.html"

As mentioned by URL parser specification the windows path separator 
happens to be a special character and hence the whole path is considered 
as a single block.
However it will be useful for the URL class to take consider "\" 
character as a path separator as a special case and be able create 
relative url which is a valid one.


I see mozilla browser simply replaces the "\" by a "/" and continues. 
URL parser could follow a similar approach.


Any thoughts ?

Re: Http client API

[Alan Bateman]

On 09/08/2012 01:49, David M. Lloyd wrote:

On 08/07/2012 06:09 PM, Michael McMahon wrote:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.


Why not javax.net.httpclient?

I assume this would require submitting it as a standalone JSR.

-Alan.

Use Builder pattern ( was: Re: Http client API)

[Chris Hegarty]

On 09/08/12 00:00, Jed Wesley-Smith wrote:

Michael McMahon  writes:


A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.


Can you separate the domain objects (in particular HttpClient, HttpRequest)
and their set-up (all the mutators) into separate concerns (Builders perhaps,
see Guava for instance)?


+1, I agree with your comment.

Wherever possible we should try to use a builder pattern to build 
immutable objects ( or limit its mutability as much as possible ). I 
think Mike made a very similar comment too. Maybe the spi and 
factory/builder could be separated out, I think this would be much cleaner.


As you say, you get thread-safety by default, which would appear to be a 
nice property for this API, given its different programming models.


-Chris.


It'd be nice to have this all thread-safe by default, it seems creating an API
that isn't thread-safe is maybe not ideal.

cheers,
Jed Wesley-Smith

Re: Http client API

[Chris Hegarty]

On 09/08/12 06:10, Sean Chou wrote:

Hi Chris,

  That's exactly my concern. I agree this provides best flexibility
and content convenient methods are not proper in this API. So, is there
going to have content specific convenient APIs like java.nio.file.Files
? Although it is a wrapper, it may be useful and intuitive, and easy to
use for those who just want to open a webpage but don't want to know
request and response. And as well doesn't break modularization. Thanks.


I think it is certainly worth some consideration, but exactly where it 
would reside and how it could be specified without "breaking" modularity 
is a bit of a conundrum. Most of the types that would be useful to have 
in such a wrapper/convenience/utility API will come from modules that 
are not from the same module as the httpclient will most likely be 
targeted to. So any API will have cross module dependencies ( not 
necessarily a problem ), just need to think through the implications.


-Chris


On Wed, Aug 8, 2012 at 5:52 PM, Chris Hegarty mailto:chris.hega...@oracle.com>> wrote:

On 08/08/2012 07:25, Sean Chou wrote:


Is it possible to have methods like

public abstract HTMLDocument getResponse(String request)  in class
HttpClient ?


Hi Sean,

I think what you are suggesting is content specific convenience
methods, something akin to URLConnection.getContent(), right? In my
experience, this type of interface can be a little problematic and
not all that widely used.

I'm personally not in favor of such convenience methods. I can see
their potential value, but this would appear to be a low level API
and I don't think it should favor one specific content type over
another.

 From my reading of the API it exposes various methods to access the
response body. It is quite easy to wrap one of these in a content
specific parser. For example, for xml you could do

   XMLInputFactory xif = ...;


xif.createStreamReader(client.__getResponse(request).__getBodyAsInputStream());

  OR
   SAXParserFactory spf = ...;


spf.newSAXParser().parse(__client.getResponse(request).__getBodyAsInputStream(),
handler);

I think this gives the best flexibility without favoring one content
type parser, or one programming model, over another.

Does this make sense? Have I missed the point of your proposal?
Maybe others have a different view...

-Chris.




On Wed, Aug 8, 2012 at 7:09 AM, Michael McMahon
mailto:michael.x.mcma...@oracle.com>
>> wrote:

 Hi,

 A new revision of the Http client API planned for jdk 8 can
be viewed
 at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/


 >

 We would like to review the api on this mailing list.
 So, all comments are welcome.

 Thanks
 Michael McMahon.




--
Best Regards,
Sean Chou




--
Best Regards,
Sean Chou

Re: Http client API

[Chris Hegarty]

Wow, Mike. Great feedback.

I just scanned your comments, and agree/understand most of them. I'll 
help Michael to feed them back into the API. Though, there'll probably 
be a few follow up mails to answers your direct questions and seek 
further clarification before the new rev of the API.


Thanks again,
-Chris.

On 08/08/12 23:33, Mike Duigou wrote:

Hi Michael!

I really look forward to using this API! It looks like you have made a lot of 
progress. Sorry for having so many comments on just one round.

Mike

General::
- It's probably already been mentioned but having the classes in the httpclient package 
and most of them begin with "Http" seems redundant.

- Package JavaDoc is missing for both packages.

- Is the SPI package needed for just one class?

- I am unsure if sendRequest should be a method of Client or Request.


HttpClientProvider::

- HttpClientProvider JavaDoc uses inconsistent capitalization of HTTP.

- createAsynchronousHttpClient() Since there's only one create method why bother to 
mention that it's "Asynchronous"?

- It's not clear how loadFromExternal() is different from provider().

- provider enumeration, request alternate providers?



HttpClient::

- There's no way to identify the source of the HttpClient(). ie. it is returned 
by provider() but there's no way to tell what you got. Would be useful for 
debugging to log to the file what provider you received.

- HttpClient createClient() -- is this the same as 
HttpClientProvider.provider().createAsynchronousHttpClient()?

- There doesn't seem to be a way to be truly thread safe about changes to 
configuration other than to set all configuration options before sharing an 
HttpClient instance and then never changing the configuration. Is changing the 
configuration *after* sharing realistic or should perhaps configuration be 
immutable (perhaps using a Builder pattern for HttpClient instances, ie. 
createClient() becomes clientBuilder()). Seeing that the set methods return 
HttpClient (the same instance unfortunately) it looks like you are considering 
or have considered using a builder style construction.

- Why expose the connection cache? Seems like just a good way to mess things 
up. Have you considered provider scope for connection cache? ie. all clients 
from the same provider share a cache?

- setProxy lacks proxy authentication features. (sorry, I just said that to 
annoy you. :-) )

- Does the proxy default to system properties and respect the nonProxyHosts? ie:
http.proxyHost (default: )
http.proxyPort (default: 80 if http.proxyHost specified)
http.nonProxyHosts (default: )

- Is the default cookie manager from CookieHandler.getDefault() or is there no 
default?

- Default is not specified for setAllowPipeLineMode() Presumably it is "false"

- There's no discussion of pipeline mode and connection cache.

- sendHeaders/sendRequest : presumably it is an error to use an HttpRequest 
from a different HttpClient.

- setResponseBodyBufferLimit - like timeout this is a default. Perhaps 
setDefaultResponseBodyBufferLimit (long I know).

- What happens with sendHeaders when the request already has a body?

- Impact of closing the OutputStream returned from sendHeaders(). chunked mode 
vs. non-chucked.

- getResponse : java.util.concurrent.TimeoutException has a nice name but is an 
odd exception to be throwing. java.net.SocketTimeoutException seems more 
appropriate.

- I'm curious why setUpgradeHandler() takes a class rather than an instance.

- getHttpsConfigurator() -- since a default is established before first 
invocation why not never return null. ie. implement the default behaviour in 
getHttpsConfigurator() and have the impl call getHttpsConfigurator().

- I missed the "s" in setHttpsConfigurator until I looked at the method in more 
detail. It doesn't really stand out.

- getFilters() -- this is the only modify-in-place API. Kinda weird. I think 
having setFilters() would be better.



HttpUpgradeHandler::

- perhaps in the spi package?

- Method or methods to identify the UpgradeHandler. ie. getName(), getProtocols.



UpgradeConnection::

- Perhaps "Upgrade**d**Connection"?

- There's no way to get back the UpdgradeHandler or source client.



SimpleConnectionClass::

- Package private?



HttpRequest::

- copy() vs clone()?

- setFollowRedirects should get a default from HttpClient (or remove defaults 
for timeout and buffer limits). Asymmetry is annoying.

- setBody(Iterable, boolean isRestartable) -- This seems heinous. 
Non-restartable Iterables should be avoided. I like the suggestion of additional 
setBody(Iterable) method instead.

- setRequestURI() -- eliminate this unless there is a really good reason to 
keep it. Alternately, perhaps copy(URI) instead?

- getMethod() -- missing.



HttpHeaders::

- getValues() how about return empty result when there is no header of that 
name?

- getValues() -- the returned list is unmodifiable, correct?

- contains() -- IAE for null.

- getFirstValue() -- IAE for null name

Problem with getFlags() method in NetworkInterface.c

[Shirish Kuncolienkar]

Hi,

The return value from the getFlags() method in NetworkInterface.c is 
interpreted in 2 ways.

- If the value is negative an Exception is thrown
- Else the return value is considered as the flag mask obtained via the 
ioctl call.


In rare cases is it possible the value in the ifr_flags could be 
negative.  One such case is VIPA interfaces.  any calls like isUp() on 
such network interfaces would end up in a Socket Exception.

I have patch for this.  Anyone would like to take a look ?

http://cr.openjdk.java.net/~luchsh/webrev20120809/

-Shirish

Re: Problem with getFlags() method in NetworkInterface.c

[Chris Hegarty]

Shirish,

I am not familiar with VIPA interfaces, but I don't see any 
documentation that describes allowable values for flags that could cause 
the integer representing it to contain a negative value.


I'm not opposed to the source changes, I just don't see that they are 
required. Can you please help explain?


Thanks,
-Chris.

On 09/08/12 11:16, Shirish Kuncolienkar wrote:

Hi,

The return value from the getFlags() method in NetworkInterface.c is
interpreted in 2 ways.
- If the value is negative an Exception is thrown
- Else the return value is considered as the flag mask obtained via the
ioctl call.

In rare cases is it possible the value in the ifr_flags could be
negative.  One such case is VIPA interfaces.  any calls like isUp() on
such network interfaces would end up in a Socket Exception.
I have patch for this.  Anyone would like to take a look ?

http://cr.openjdk.java.net/~luchsh/webrev20120809/

-Shirish

Re: Problem with getFlags() method in NetworkInterface.c

[Shirish Kuncolienkar]

On 8/9/2012 4:55 PM, Chris Hegarty wrote:

Shirish,

I am not familiar with VIPA interfaces, but I don't see any 
documentation that describes allowable values for flags that could 
cause the integer representing it to contain a negative value.


I'm not opposed to the source changes, I just don't see that they are 
required. Can you please help explain?


Thanks,
-Chris.

On 09/08/12 11:16, Shirish Kuncolienkar wrote:

Hi,

The return value from the getFlags() method in NetworkInterface.c is
interpreted in 2 ways.
- If the value is negative an Exception is thrown
- Else the return value is considered as the flag mask obtained via the
ioctl call.

In rare cases is it possible the value in the ifr_flags could be
negative.  One such case is VIPA interfaces.  any calls like isUp() on
such network interfaces would end up in a Socket Exception.
I have patch for this.  Anyone would like to take a look ?

http://cr.openjdk.java.net/~luchsh/webrev20120809/

-Shirish




Chris,

I agree there is no general documentation available, AIX defines vipa 
interface flag as "0x8000"
Here is a similar bug report related to FreeBSD 
http://hg.openjdk.java.net/macosx-port/macosx-port/jdk/rev/c6334146005c.

A different fix was proposed here.

Hope this helps.

Thanks
-Shirish

Re: Problem with getFlags() method in NetworkInterface.c

[Chris Hegarty]

On 09/08/12 14:16, Shirish Kuncolienkar wrote:


I agree there is no general documentation available, AIX defines vipa
interface flag as "0x8000"


In which case I don't see any problems with your proposed source 
changes. One could argue that maybe they should go through the specific 
porting project ( since it's not directly relevant to existing supported 
platforms ), but I see this more of a clean up exercise.  No need to 
carry such a trivial change in a project sub repo.


Usually a new testcase is recommended, but in this case the 
functionality ( isUp, isXXX() ) is already exercised by many many tests 
so I think we can leave it to the other tests.


I filed a new bug to track this issue,
   CR 7190254: "NetworkInterface getFlags implementation should support
full integer bit range for flags value."

If you don't mind, I would like to take your patch to NetworkInterface.c 
and run some sanity builds and tests on it. I'd hope to get this done 
later today or early tomorrow.


-Chris.


Here is a similar bug report related to FreeBSD
http://hg.openjdk.java.net/macosx-port/macosx-port/jdk/rev/c6334146005c.
A different fix was proposed here.

Hope this helps.

Thanks
-Shirish

Re: Problem with getFlags() method in NetworkInterface.c

[Shirish Kuncolienkar]

Chris,

Please go ahead and run the sanity builds and tests.

Thanks
-Shirish

On 8/9/2012 7:52 PM, Chris Hegarty wrote:


On 09/08/12 14:16, Shirish Kuncolienkar wrote:


I agree there is no general documentation available, AIX defines vipa
interface flag as "0x8000"


In which case I don't see any problems with your proposed source 
changes. One could argue that maybe they should go through the 
specific porting project ( since it's not directly relevant to 
existing supported platforms ), but I see this more of a clean up 
exercise.  No need to carry such a trivial change in a project sub repo.


Usually a new testcase is recommended, but in this case the 
functionality ( isUp, isXXX() ) is already exercised by many many 
tests so I think we can leave it to the other tests.


I filed a new bug to track this issue,
   CR 7190254: "NetworkInterface getFlags implementation should support
full integer bit range for flags value."

If you don't mind, I would like to take your patch to 
NetworkInterface.c and run some sanity builds and tests on it. I'd 
hope to get this done later today or early tomorrow.


-Chris.


Here is a similar bug report related to FreeBSD
http://hg.openjdk.java.net/macosx-port/macosx-port/jdk/rev/c6334146005c.
A different fix was proposed here.

Hope this helps.

Thanks
-Shirish

hg: jdk8/tl/jdk: 7189363: Regex Pattern compilation buggy for special sequences

[xueming . shen]

Changeset: 717ed00b7787
Author:sherman
Date:  2012-08-09 10:15 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/717ed00b7787

7189363: Regex Pattern compilation buggy for special sequences
Summary: fixed the incorrect implementation in expr(...)
Reviewed-by: psandoz, alanb

! src/share/classes/java/util/regex/Pattern.java
! test/java/util/regex/RegExTest.java

Re: Http client API

[Chris Hegarty]

Michael,

Looking good, some comments.

1) Why the use of CookieManager, rather than CookieHandler?  I  would
   expect that CookieHandler would be a cleaner API

2) What is the impact on the sendHeader, setBody for HEAD requests?

3) I think HttpClient could be an interface and move the create method
   to a builder/factory, and make it as immutable as possible ( this
   came up a few times now ).

4) The Filter API looks a little funny, in that filter instances are
   added to the client, while the ByteBufferWrapper instances are
   presumably created by the implementation after registering the
   wrapper class with the filter instance. Probably best/cleaner
   to use the same style. It could also be an interface.

5) ByteBufferWrapper seems a little cluttered with implementation detail
   setSource() nor setWrapper()?? Maybe best to just provide an
   interface.

6) Upgrade handler, similar comment to 4. Why not just register an
   instance.

7) Exposing the filter list seems a little wrong, given the getter/
   setter style.

8) java.net.httpclient.HttpConnectionCache.CachedConnection should
   probably be an interface.

9) How does the cache handle tunnels? endpoint address/proxy/etc

10) Missing fluent style return from HttpRequest.setRequestBodyLimit

11) Should sendHeaders be specified to allow a null return value. I'm
thinking about when setSendExpectContinue is set.

-Chris.


On 08/08/12 00:09, Michael McMahon wrote:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.

Thanks
Michael McMahon.