Re: JDK FTP support

[Jean-Christophe Collet]

Thank you for proposition, we certainly will take a look a all that.
That being said, the FTP Client API project is in a unusual state.
You see, as a pointed numerous times, we do have an API already, even if 
it's a rather incomplete and not flexible one. Which leads to 2 issues:
1) Any new FTP API has to be backward compatible with the existing one, 
and, to avoid bloat, should reuse most of the existing one.
2) So far it has been the blocking argument when trying to convince "the 
powers that be" that a new API is needed. I've been trying to get that 
project approved for a long time now (years to be exact). As of today it 
is still not approved for jdk7, even though I have good hopes that it 
will be.


The corollary to that is that the API, and implementation, for a full 
FTP client has been in the works, internally, for a long time. 
Basically, it's done, just waiting in one of our internal workspaces for 
final approval.


Now, assuming the project finally gets approved, I fully intend to 
publish what we've done before committing it to the workspace in order 
to get feedback, reviews, and, hopefully, contributions.
At which point, I will take a look at your work and see what can be done 
to benefits all. In particular anything that could help with unit tests 
will be very, very needed.


I hope this clarify a few things concerning FTP support in JDK 7.
Don't hesitate to contact me on that subject, or any networking issue, 
if you have questions or issues.


Thanks again,

David Hansmann wrote:

Hi all,

I recently looked at openjdk.java.net and found that there doesn't 
seem to be a fixed plan to integrate full FTP support
(I'm referring to 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4650689 ), so I 
wonder if you might be interested to use parts

of my FTP API located on http://j-ftp.sourceforge.net .

It  is part of a file transfer client which supports FTP using an API 
which I wrote and which has been tested for quite some time now,
SFTP (throug j2ssh or jsch), JCIFS (through JCIFS) and NFS (through 
Sun WebNFS) using
a common interface. Although the code may not be state-of-the art (no 
generics yet) the core classes are very small, easy to refactor, not 
very complicated in general and there are various of the features you 
mentioned already present (like one control connection for multiple 
downloads,

callbacks) and more.

It should for example be possible to to use the other APIs as plugins 
once this would be implemented so those other protocols could be used

by just placing the jars anywhere in the classpath.

I'd of course help with the developement, code cleanup, testing and 
support if anybody wants to give it a try - it would be really exiting to
contribute to the JDK especially because I started the project years 
ago because of the lack of a real (and OSS) java FTP API ;)


Greetings and thanks for your time,

David Hansmann

P.S.: If you want to take a look at the code you probably want to look 
at net/FtpConnection, the BasicConnection interface and the example code

in the doc/-directory.

Proposed fix for 6660405.

[Jean-Christophe Collet]

Here is my proposed fix for CR 6660405.
The issue is that after a redirect, and if there is a cache hit (i.e. 
the URL is found in the cache) HttpURLConnection will return the wrong 
InputStream. Details are in the evaluation section of the CR.
My proposed fix is to set inputStream back to null in 
sun.net.www.protocol.http.HttpURLConnection.disconnectInternal().

I've also included a regression test that reproduces the bug.
See patch attached.

--- old/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java	2008-03-05 16:05:35.0 +0100
+++ new/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java	2008-03-05 16:05:35.0 +0100
@@ -2048,6 +2048,7 @@
  */
 private void disconnectInternal() {
 responseCode = -1;
+inputStream = null;
 if (pi != null) {
 pi.finishTracking();
 pi = null;
--- /dev/null	2008-02-29 10:40:07.0 +0100
+++ new/test/sun/net/www/protocol/http/B6660405.java	2008-03-05 16:05:36.0 +0100
@@ -0,0 +1,165 @@
+/*
+ * Copyright 2008 Sun Microsystems, Inc.  All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/*
+ * @test
+ * @bug 6660405
+ * @summary HttpURLConnection returns the wrong InputStream
+ */
+
+import java.net.*;
+import java.util.*;
+import java.io.*;
+import com.sun.net.httpserver.*;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ExecutorService;
+
+public class B6660405
+{
+com.sun.net.httpserver.HttpServer httpServer;
+ExecutorService executorService;
+
+static class MyCacheResponse extends CacheResponse {
+private byte[] buf = new byte[1024];
+
+public MyCacheResponse() {
+
+}
+
+@Override
+public Map> getHeaders() throws IOException
+{
+Map> h = new HashMap>();
+ArrayList l = new ArrayList();
+l.add("HTTP/1.1 200 OK");
+h.put(null, l);
+l = new ArrayList();
+l.add("1024");
+h.put("Content-Length", l);
+return h;
+}
+
+@Override
+public InputStream getBody() throws IOException
+{
+return new ByteArrayInputStream(buf);
+}
+
+}
+static class MyResponseCache extends ResponseCache {
+
+public MyResponseCache() {
+
+}
+
+@Override
+public CacheResponse get(URI uri, String rqstMethod, Map> rqstHeaders) throws IOException
+{
+if (uri.getPath().equals("/redirect/index.html")) {
+return new MyCacheResponse();
+}
+return null;
+}
+
+@Override
+public CacheRequest put(URI uri, URLConnection conn) throws IOException
+{
+return null;
+}
+
+}
+ 
+public static void main(String[] args)
+{
+new B6660405();
+}
+
+public B6660405()
+{
+try {
+startHttpServer();
+doClient();
+} catch (IOException ioe) {
+System.err.println(ioe);
+}
+}
+
+void doClient() {
+ResponseCache.setDefault(new MyResponseCache());
+try {
+InetSocketAddress address = httpServer.getAddress();
+
+// GET Request
+URL url = new URL("http://localhost:"; + address.getPort() + "/test/index.html");
+HttpURLConnection uc = (HttpURLConnection)url.openConnection();
+int code = uc.getResponseCode();
+System.err.println("response code = " + code);
+int l = uc.getContentLength();
+System.err.println("content-length = " + l);
+InputStream in = uc.getInputStream();
+int i = 0;
+// Read till end of stream
+do {
+i = in.read();
+} while (i != -1);
+in.close();
+} catch (IOException e) {
+throw new RuntimeException("Got the wrong

Proposed fix for 6651717

[Jean-Christophe Collet]

This is a straightforward fix in mailtoURLConnection to remove a left 
over debug statement in the connect() method.
Also took the opportunity to remove unneeded imports and add appropriate 
@Override tags.

No regression test as this is a trivial change.
Patch attached.

--- old/src/share/classes/sun/net/www/protocol/mailto/MailToURLConnection.java	2008-03-05 17:37:36.0 +0100
+++ new/src/share/classes/sun/net/www/protocol/mailto/MailToURLConnection.java	2008-03-05 17:37:35.0 +0100
@@ -29,9 +29,6 @@
 import java.net.InetAddress;
 import java.net.SocketPermission;
 import java.io.*;
-import java.util.Enumeration;
-import java.util.Hashtable;
-import java.util.StringTokenizer;
 import java.security.Permission;
 import sun.net.www.*;
 import sun.net.smtp.SmtpClient;
@@ -86,11 +83,11 @@
 }
 
 public void connect() throws IOException {
-System.err.println("connect. Timeout = " + connectTimeout);
 client = new SmtpClient(connectTimeout);
 client.setReadTimeout(readTimeout);
 }
 
+@Override
 public synchronized OutputStream getOutputStream() throws IOException {
 if (os != null) {
 return os;
@@ -107,6 +104,7 @@
 return os;
 }
 
+@Override
 public Permission getPermission() throws IOException {
 if (permission == null) {
 connect();
@@ -116,22 +114,26 @@
 return permission;
 }
 
+@Override
 public void setConnectTimeout(int timeout) {
 if (timeout < 0)
 throw new IllegalArgumentException("timeouts can't be negative");
 connectTimeout = timeout;
 }
 
+@Override
 public int getConnectTimeout() {
 return (connectTimeout < 0 ? 0 : connectTimeout);
 }
 
+@Override
 public void setReadTimeout(int timeout) {
 if (timeout < 0)
 throw new IllegalArgumentException("timeouts can't be negative");
 readTimeout = timeout;
 }
 
+@Override
 public int getReadTimeout() {
 return readTimeout < 0 ? 0 : readTimeout;
 }

Re: CR 6667108 code review

[Jean-Christophe Collet]

Approved.

Christopher Hegarty - Sun Microsystems Ireland wrote:
CR 6667108: "typo in javadoc for 
java.net.Socket.getRemoteSocketAddress()"


Trivial cleanup of a typo in getRemoteSocketAddress specification.


diff -r fa6948bdc4b0 src/share/classes/java/net/Socket.java
--- a/src/share/classes/java/net/Socket.javaThu Mar 06 10:35:28 
2008 -0800
+++ b/src/share/classes/java/net/Socket.javaFri Mar 07 09:51:14 
2008 +

@@ -731,7 +731,7 @@ class Socket implements java.io.Closeabl
  * then this method will continue to return the connected address
  * after the socket is closed.
  *
- * @return a SocketAddress reprensenting the remote 
endpoint of this
+ * @return a SocketAddress representing the remote 
endpoint of this

  * socket, or null if it is not connected yet.
  * @see #getInetAddress()
  * @see #getPort()

-Chris.

Re: CR 6591358 code review

[Jean-Christophe Collet]

Approved.

Christopher Hegarty - Sun Microsystems Ireland wrote:

Another trivial documentation change.

CR 6591358:
 "documentation error in URLConnection.setRequestProperty("accept", ...)"

-Chris.

HttpCookie, InMemoryCookieStore and domain matching

[Jean-Christophe Collet]

I filed CR 6644726 (Cookie management issues) after discovering a few 
inconsistencies between the way J2SE handles cookies and the 'real 
world'. I also have been working on a fix for that.


I thought all these issues were bugs, but one turned out to be a more 
complicated.
It's actually the first one mentioned in the CR: the way domains are 
matched.
In short, the actual code will not send cookies from the 'foo.com' 
domain to the 'xxx.yyy.foo.com' site.

Which breaks a few sites, like Yahoo.
I initially thought that HttpCookie.domainMatches() was the culprit, but 
I discovered that it actually follows RFC 2965 sec. 3.3.2 to the letter 
as specified in the Javadoc.

So I don't think changing that code is the answer.
However, it should be noted that, in theory, that domain matching should 
only used for RFC 2965 conform cookies. 'Old' cookies, as in the 
netscape draft should behave differently.
After more research and consideration I would suggest to change our 
implementation of CookieStore 
(sun.net.www.protocol.http.InMemoryCookieStore) to take the cookie 
version (as returned by HttpCookie.getVersion) when doing the domain 
matching. In the case of a RFC 2965 cookie (version == 1) we would still 
use HttpCookie.domainMatches(), in the case of a 'Netscape' cookie 
(version == 0), we would use a slightly different comparison to allow 
for the aforementioned situation.


I will produce proposed code changes soon, but I'd like to hear any 
thoughts on that subject.

Re: SocketImpl.create()

[Jean-Christophe Collet]

Roman Kennke wrote:

I am confused how this method is supposed to work. It has a boolean
parameter that is documented to be used to distinguish between datagram
and stream sockets. However, it is only used by Socket and ServerSocket,
so I guess it must always be a stream socket? But it gets worse. The
PlainSocketImpl.create() calls socketCreate() with that parameter, where
it is documented to be used to distinguish between server and client
socket (is there a difference??). Looking into the native code for that
thought, this method calls JVM_Socket() with that parameter to
distinguish between datagram and stream sockets. Something does not fit
here... Is this parameter used at all?

Cheers,
/Roman
  
That parameter is a left over of a very early version of the 
Socket/SocketImpl API.

It is now obsolete mostly and was left for backward compatibility reasons.
In, short, no, it's not used anymore since all SocketImpls are now TCP 
based.

Code review for 6644726: Cookie management issues

[Jean-Christophe Collet]

Here are my proposed changes to fix 6644726
It does fix all 7 issues listed in the CR (see 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6644726 for details) 
and adds a proper regression test.


The short list is:

- support for more 'expires' date format
- enforce 'secure' cookies
- enforce the 'port' optional attribute
- set a default 'path' when none is specified as per RFC requirement
- do not apply strict RFC 2965 rules for domain matching when cookie 
version is 0 (aka Netscape compliant cookies)

- set a default path for cookies
- do not use scheme and port for identifying cookies (i.e. cookies cross 
over protocols like http & https, or ports)


Patch attached below.

--- a/src/share/classes/java/net/CookieManager.java	Wed Apr 02 22:44:45 2008 -0400
+++ b/src/share/classes/java/net/CookieManager.java	Tue Apr 15 13:21:02 2008 +0200
@@ -205,11 +205,31 @@ public class CookieManager extends Cooki
 if (cookieJar == null)
 return Collections.unmodifiableMap(cookieMap);
 
+boolean secureLink = "https".equalsIgnoreCase(uri.getScheme());
 List cookies = new java.util.ArrayList();
+String path = uri.getPath();
+if (path == null || path.isEmpty()) {
+path = "/";
+}
 for (HttpCookie cookie : cookieJar.get(uri)) {
 // apply path-matches rule (RFC 2965 sec. 3.3.4)
-if (pathMatches(uri.getPath(), cookie.getPath())) {
-cookies.add(cookie);
+// and check for the possible "secure" tag (i.e. don't send
+// 'secure' cookies over unsecure links)
+if (pathMatches(path, cookie.getPath()) &&
+(secureLink || !cookie.getSecure())) {
+// Let's check the authorize port list if it exists
+String ports = cookie.getPortlist();
+if (ports != null && !ports.isEmpty()) {
+int port = uri.getPort();
+if (port == -1) {
+port = "https".equals(uri.getScheme()) ? 443 : 80;
+}
+if (isInPortList(ports, port)) {
+cookies.add(cookie);
+}
+} else {
+cookies.add(cookie);
+}
 }
 }
 
@@ -251,8 +271,46 @@ public class CookieManager extends Cooki
 try {
 List cookies = HttpCookie.parse(headerValue);
 for (HttpCookie cookie : cookies) {
-if (shouldAcceptInternal(uri, cookie)) {
-cookieJar.add(uri, cookie);
+if (cookie.getPath() == null) {
+// If no path is specified, then by default
+// the path is the directory of the page/doc
+String path = uri.getPath();
+if (!path.endsWith("/")) {
+int i = path.lastIndexOf("/");
+if (i > 0) {
+path = path.substring(0, i + 1);
+} else {
+path = "/";
+}
+}
+cookie.setPath(path);
+}
+String ports = cookie.getPortlist(); 
+if (ports != null) {
+int port = uri.getPort();
+if (port == -1) {
+port = "https".equals(uri.getScheme()) ? 443 : 80;
+}
+if (ports.isEmpty()) {
+// Empty port list means this should be restricted
+// to the incoming URI port
+cookie.setPortlist("" + port );
+if (shouldAcceptInternal(uri, cookie)) {
+cookieJar.add(uri, cookie);
+}
+} else {
+// Only store cookies with a port list
+// IF the URI port is in that list, as per
+// RFC 2965 section 3.3.2
+if (isInPortList(ports, port) &&
+shouldAcceptInternal(uri, cookie)) {
+cookieJar.add(uri, cookie);
+}
+}
+} else {
+if (shouldAcceptInternal(uri, cookie)) {
+cookieJar.add(uri, cookie);
+}
 }
 }
 } catch (IllegalArgumentException e) {
@@ -275,6 +333,32 @@ 

Re: 6659779 code review

[Jean-Christophe Collet]

Approved.

Christopher Hegarty - Sun Microsystem wrote:

Hi Michael, Jessie,

Can I please get a code review for CR 6659779. This trivially adds 
logger invocations to log tunnel requests, ie. HTTP CONNECT.


-Chris.

hg: jdk7/jsn/jdk: 6644726: Cookie management issues

[jean-christophe . collet]

Changeset: d44e3bf49ffb
Author:jccollet
Date:  2008-04-17 11:05 +0200
URL:   http://hg.openjdk.java.net/jdk7/jsn/jdk/rev/d44e3bf49ffb

6644726: Cookie management issues
Summary: Many changes to accomodate RFC 2965 and old Netscape specs
Reviewed-by: chegar

! src/share/classes/java/net/CookieManager.java
! src/share/classes/java/net/HttpCookie.java
! src/share/classes/sun/net/www/protocol/http/InMemoryCookieStore.java
+ test/java/net/CookieHandler/B6644726.java

Code review needed for 6558853

[Jean-Christophe Collet]

I need a code review for the simple fix to 6558853.
It's a simple case of a flag not set in the native code when creating a 
Inet6Address.


Patch attached below.

Thanks
--- /home/jccollet/tmp/webrev/raw_files/old/src/share/native/java/net/net_util.c	2008-04-18 13:59:44.0 +0200
+++ /home/jccollet/tmp/webrev/raw_files/new/src/share/native/java/net/net_util.c	2008-04-18 13:59:44.0 +0200
@@ -112,6 +112,7 @@
 (*env)->SetIntField(env, iaObj, ia_familyID, IPv4);
 } else {
 static jclass inet6Cls = 0;
+jint scope;
 if (inet6Cls == 0) {
 jclass c = (*env)->FindClass(env, "java/net/Inet6Address");
 CHECK_NULL_RETURN(c, NULL);
@@ -129,7 +130,10 @@
 (*env)->SetObjectField(env, iaObj, ia6_ipaddressID, ipaddress);
 
 (*env)->SetIntField(env, iaObj, ia_familyID, IPv6);
-(*env)->SetIntField(env, iaObj, ia6_scopeidID, getScopeID(him));
+scope = getScopeID(him);
+(*env)->SetIntField(env, iaObj, ia6_scopeidID, scope);
+if (scope > 0)
+(*env)->SetBooleanField(env, iaObj, ia6_scopeidsetID, JNI_TRUE);
 }
 *port = ntohs(him6->sin6_port);
 } else

--- new/test/java/net/Inet6Address/B6558853.java	2008-04-18 13:59:44.0 +0200
+++ /dev/null	2008-02-29 10:40:07.0 +0100
@@ -1,86 +0,0 @@
-/*
- * Copyright 2008 Sun Microsystems, Inc.  All Rights Reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
- * CA 95054 USA or visit www.sun.com if you need additional information or
- * have any questions.
- */
-
-/**
- * @test
- * @bug 6558853
- * @summary  getHostAddress() on connections using IPv6 link-local addrs should have zone id
- */
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.net.*;
-import java.util.Enumeration;
-
-public class B6558853 implements Runnable {
-private InetAddress addr = null;
-private int port = 0;
-
-public static void main(String[] args) throws Exception {
-ServerSocket ss = new ServerSocket(0);
-int port = ss.getLocalPort();
-Enumeration l = NetworkInterface.getNetworkInterfaces();
-InetAddress dest = null;
-while (l.hasMoreElements() && dest == null) {
-NetworkInterface nif = l.nextElement();
-for (InterfaceAddress a : nif.getInterfaceAddresses()) {
-if (a.getAddress() instanceof Inet6Address) {
-Inet6Address a6 = (Inet6Address) a.getAddress();
-if (a6.isLinkLocalAddress()) {
-dest = a6;
-}
-break;
-}
-}
-}
-if (dest != null) {
-B6558853 test = new B6558853(dest, port);
-Thread thread = new Thread(test);
-thread.start();
-Socket s = ss.accept();
-InetAddress a = s.getInetAddress();
-OutputStream out = s.getOutputStream();
-out.write(1);
-out.close();
-if (!(a instanceof Inet6Address) || a.getHostAddress().indexOf("%") == -1) {
-// No Scope found in the address String
-throw new RuntimeException("Wrong address: " + a.getHostAddress());
-}
-}
-}
-
-public B6558853(InetAddress a, int port) {
-addr = a;
-this.port = port;
-}
-
-public void run() {
-try {
-Socket s = new Socket(addr, port);
-InputStream in = s.getInputStream();
-int i = in.read();
-in.close();
-} catch (IOException iOException) {
-}
-}
-}

hg: jdk7/jsn/jdk: 6558853: getHostAddress() on connections using IPv6 link-local addrs should have zone id

[jean-christophe . collet]

Changeset: a71ab67d3ece
Author:jccollet
Date:  2008-04-18 15:23 +0200
URL:   http://hg.openjdk.java.net/jdk7/jsn/jdk/rev/a71ab67d3ece

6558853: getHostAddress() on connections using IPv6 link-local addrs should 
have zone id
Summary: Set the scope_id_set flag when necessary
Reviewed-by: chegar

! src/share/native/java/net/net_util.c
+ test/java/net/Inet6Address/B6558853.java

A proposal for a FTP client API

[Jean-Christophe Collet]

Hello,

I have posted an entry in my blog about the current status of the FTP 
client API.
It contains a quick description of the project as well as a link to the 
current draft of the API. So if you're interested in that topic go take 
a look at http://blogs.sun.com/jcc/


As mentioned in the post, feedback is very strongly encouraged.

Re: A proposal for a FTP client API

[Jean-Christophe Collet]

Alan Bateman wrote:

Jean-Christophe Collet wrote:

Hello,

I have posted an entry in my blog about the current status of the FTP 
client API.
It contains a quick description of the project as well as a link to 
the current draft of the API. So if you're interested in that topic 
go take a look at http://blogs.sun.com/jcc/


As mentioned in the post, feedback is very strongly encouraged.


I'm interested in reviewing this. From an initial glance this is ftp 
and ftps - are you also thinking about sftp? Some initial comments 
from a first pass:


It would be better to use a static factory method rather than public 
constructors. For one thing, there are a slew of ftp clients about and 
you'll likely get requests for a provider interface so that 
alternative implementations can be used.


Good point. Current design inherited a lot from the existing 
sun.net.ftp.FtpClient code (for obvious compatibility reasons), but 
that's definitely one thing we can improve.




The client has state (not connected, connected, logged, etc.) and you 
might want to introduce a few specific exceptions  for this. For 
example, what does connect throw if you are already connected, what do 
the other methods throw when you aren't connected, etc.


In current draft there are quite a few methods that return a boolean 
and throw IOException. It's not clear to me what what failure means - 
do these methods return false or do they throw an I/O exception? How 
is this used with the getLastReplyCode method or should the exception 
have a method to return the reply code?




The idea is to differentiate errors reported by the FTP server (aka non 
fatal errors), like 'bad login' or file not found, from the serious 
networking errors (connection lost, no route to host etc...).
In the first case, the current connection can still be used for other 
attempts, while in the second case it's more likely too serious to recover.


The listFiles returns a List and isn't going to scale with 
large directories (you'll have to fetch the entire directory from the 
server for this method to return). It would be better to return 
something that implements Iterable and Closeable.




This seems a common concern. I'll update the API accordingly.

I'm curious about FtpFile. My memory of the ftp protocol is hazy but I 
thought that commands such as LIST or NLIST didn't specify the format 
of the file attributes. There was a IEFT WG working on defining these 
but I don't see this in the references. Do you have a pointer to that?


They don't. That's why the API provides a pluggable 'directory parser'. 
By default we provide a parser that is able to 'grok' most of what is 
out there, but in case it's needed, the application can provide its own.




I see that the getModificationTime returns a Date. Care to revise your 
statement sir :-)



Shows how long this API has been in the working
I will change that.

You've got a number of setter methods that return void. It might be 
nicer for them to return this so as to facilitate invocation chaining.




Good point.


Should the members of the TransferMode enum be in uppercase?



Probably.
Same for TransferType (and I probably should remove EBCDIC since it's 
not really supported).


Do you really need the ability to set/get the connect timeout and 
allow it be overridden by the connect method?




I guess this is a bit redundant. Will clarify.


That's it for a first pass.


Thanks,

Re: A proposal for a FTP client API

[Jean-Christophe Collet]

Max (Weijun) Wang wrote:

boolean login(String user, String password)
boolean login(String user, String password, String account)

Normally we use char[] to represent a password so that it can 
zeroed out afterward.




Good point, I'll make the necessary changes.


List listFiles(String path)

If a FTP directory is huge, listing the files will cost a very 
long time. I'll be very glad to see this method returns immediately 
and fetching the information while user iterate through the output. Is 
this implementation possible with this API?


Not with the current design. However, it is possible to get the 'raw' 
listing using FtpClient.list(String path) instead. It returns an 
InputStream to the unparsed listing.




Thanks
Max

On May 23, 2008, at 11:20 PM, Jean-Christophe Collet wrote:

Hello,

I have posted an entry in my blog about the current status of the FTP 
client API.
It contains a quick description of the project as well as a link to 
the current draft of the API. So if you're interested in that topic 
go take a look at http://blogs.sun.com/jcc/


As mentioned in the post, feedback is very strongly encouraged.

Re: A proposal for a FTP client API

[Jean-Christophe Collet]

David M. Lloyd wrote:

On 05/23/2008 10:20 AM, Jean-Christophe Collet wrote:

Hello,

I have posted an entry in my blog about the current status of the FTP 
client API.
It contains a quick description of the project as well as a link to 
the current draft of the API. So if you're interested in that topic 
go take a look at http://blogs.sun.com/jcc/


As mentioned in the post, feedback is very strongly encouraged.


This is going into the java.net package - is there/will there be a JSR 
for this effort?


Not a separate JSR no, it is too small of a change to justify its own 
JSR. However it will be covered by the JDK7 blanket JSR.

FTP client API draft updated

[Jean-Christophe Collet]

I've updated the API draft to cover most of the points raised here.

See http://blogs.sun.com/jcc/ for the details.

Thanks for the feedback, past and future :-)

Proposed changes for 6656849

[Jean-Christophe Collet]

This is a serialization issue in Inet6Address. When de-serializing an 
address with an interface name as the scope, the test for its existence 
should be done sooner rather than later to avoid a NullPointerException.
I took the opportunity to clean up the code in Inet6Address to act on 
some NetBeans warning, in particular re-named a local variable that was 
masking a field.



See patch attached.

--- /home/jccollet/tmp/webrev/raw_files/old/src/share/classes/java/net/Inet6Address.java	2008-06-16 17:14:55.0 +0200
+++ /home/jccollet/tmp/webrev/raw_files/new/src/share/classes/java/net/Inet6Address.java	2008-06-16 17:14:55.0 +0200
@@ -25,12 +25,9 @@
 
 package java.net;
 
-import java.security.AccessController;
 import java.io.ObjectInputStream;
 import java.io.IOException;
-import java.io.ObjectStreamException;
 import java.io.InvalidObjectException;
-import sun.security.action.*;
 import java.util.Enumeration;
 
 /**
@@ -360,11 +357,11 @@
 private int deriveNumericScope (NetworkInterface ifc) throws UnknownHostException {
 Enumeration addresses = ifc.getInetAddresses();
 while (addresses.hasMoreElements()) {
-InetAddress address = (InetAddress)addresses.nextElement();
-if (!(address instanceof Inet6Address)) {
+InetAddress addr = (InetAddress)addresses.nextElement();
+if (!(addr instanceof Inet6Address)) {
 continue;
 }
-Inet6Address ia6_addr = (Inet6Address)address;
+Inet6Address ia6_addr = (Inet6Address)addr;
 /* check if site or link local prefixes match */
 if (!differentLocalAddressTypes(ia6_addr)){
 /* type not the same, so carry on searching */
@@ -388,11 +385,11 @@
 if (ifc.getName().equals (ifname)) {
 Enumeration addresses = ifc.getInetAddresses();
 while (addresses.hasMoreElements()) {
-InetAddress address = (InetAddress)addresses.nextElement();
-if (!(address instanceof Inet6Address)) {
+InetAddress addr = (InetAddress)addresses.nextElement();
+if (!(addr instanceof Inet6Address)) {
 continue;
 }
-Inet6Address ia6_addr = (Inet6Address)address;
+Inet6Address ia6_addr = (Inet6Address)addr;
 /* check if site or link local prefixes match */
 if (!differentLocalAddressTypes(ia6_addr)){
 /* type not the same, so carry on searching */
@@ -420,21 +417,22 @@
 if (ifname != null && !"".equals (ifname)) {
 try {
 scope_ifname = NetworkInterface.getByName(ifname);
-try {
-scope_id = deriveNumericScope (scope_ifname);
-} catch (UnknownHostException e) {
-// should not happen
-assert false;
+if (scope_ifname == null) {
+/* the interface does not exist on this system, so we clear
+ * the scope information completely */
+scope_id_set = false;
+scope_ifname_set = false;
+scope_id = 0;
+} else {
+try {
+scope_id = deriveNumericScope (scope_ifname);
+} catch (UnknownHostException e) {
+// should not happen
+assert false;
+}
 }
 } catch (SocketException e) {}
 
-if (scope_ifname == null) {
-/* the interface does not exist on this system, so we clear
- * the scope information completely */
-scope_id_set = false;
-scope_ifname_set = false;
-scope_id = 0;
-}
 }
 /* if ifname was not supplied, then the numeric info is used */
 
@@ -460,6 +458,7 @@
  * an IP multicast address
  * @since JDK1.1
  */
+@Override
 public boolean isMulticastAddress() {
 return ((ipaddress[0] & 0xff) == 0xff);
 }
@@ -470,6 +469,7 @@
  * a wildcard address.
  * @since 1.4
  */
+@Override
 public boolean isAnyLocalAddress() {
 byte test = 0x00;
 for (int i = 0; i < INADDRSZ; i++) {
@@ -485,6 +485,7 @@
  * a loopback address; or false otherwise.
  * @since 1.4
  */
+@Override
 public boolean isLoopbackAddress() {
 byte test = 0x00;
 for (int i = 0; i < 15; i++) {
@@ -500,6 +501,7 @@
  * a link local address; or false if address is not a link local unicast address.
  * @since 1.4
  */
+@Override
 public boolean isLinkLocalAddress() {
 return ((ipaddress[0] & 0xff) == 0xfe
 && (ipaddress[1] & 0xc0) == 0x80);
@@ -512,6 +514,7 

hg: jdk7/jsn/jdk: 6717876: Make java.net.NetworkInterface.getIndex() public

[jean-christophe . collet]

Changeset: f4289d75cd29
Author:jccollet
Date:  2008-08-25 14:38 +0200
URL:   http://hg.openjdk.java.net/jdk7/jsn/jdk/rev/f4289d75cd29

6717876: Make java.net.NetworkInterface.getIndex() public
Summary: Make getIndex() and getByIndex() public. Required a name change in 
native code
Reviewed-by: alanb, chegar, michaelm

! make/java/net/mapfile-vers
! src/share/classes/java/net/NetworkInterface.java
! src/solaris/native/java/net/NetworkInterface.c
! src/solaris/native/java/net/PlainDatagramSocketImpl.c
! src/windows/native/java/net/NetworkInterface.c
! src/windows/native/java/net/NetworkInterface_winXP.c
! src/windows/native/java/net/TwoStacksPlainDatagramSocketImpl.c
! src/windows/native/java/net/net_util_md.h
+ test/java/net/NetworkInterface/IndexTest.java

Re: hg: jdk7/jsn/jdk: 6717876: Make java.net.NetworkInterface.getIndex() public

[Jean-Christophe Collet]

Andrew John Hughes wrote:

On 27/08/2008, Mark Wielaard <[EMAIL PROTECTED]> wrote:
  

Hi,


 On Mon, Aug 25, 2008 at 12:43:59PM +, [EMAIL PROTECTED] wrote:
 > URL:   http://hg.openjdk.java.net/jdk7/jsn/jdk/rev/f4289d75cd29
 >
 > 6717876: Make java.net.NetworkInterface.getIndex() public
 > Summary: Make getIndex() and getByIndex() public. Required a name change in 
native code
 > Reviewed-by: alanb, chegar, michaelm


This seems to introduce a new public API in the java.net package.
 I was wondering how that works for someone that wants to introduce
 such things in openjdk. I do see the bug report that mentions making
 this public should be done for JDK7. But there seems to missing a real
 justification for adding these new interfaces. Is there any policy for
 introducing such new public interfaces?

 I might be reading this wrongly but it seems the only thing this API
 does is expose some random internal "index" numbers for an
 NetworkInterface. Seeing that there are no guarantees on whether there
 are index numbers in the first place for any or all interfaces it seems
 not that useful imho. An example of how an application would use this
 new interface would be nice to get a better idea in what situations
 this would be used.

 Thanks,


 Mark




I also thought the change was odd, given it adds two methods based on
otherwise undefined 'system specific' values.  From the methods given,
there is no information as to how many of these index numbers are in
use.  The only way I can see to enumerate them is to get all instances
of NetworkInterface and call getIndex() on each.  I assume that the
application of these methods is to resolve ambiguity between two
interfaces with the same name but a different index, but it would be
more helpful if the Javadoc included example usage and some
information on what these numbers might be.

More generally, it's not clear where the decision was made to make
this part of the (as yet non-existent) JDK7 platform JSR.  Will there
be more insight into this process from outside Sun in the near future


A network interface index is hardly a "random internal number".
This is a value attributed by the operating system and is as significant 
as an interface name.
The reason it was made public is because it's an information that is 
needed by other APIs (like NIO) and applications.
An example is that such an index can be used inside an IPv6 address to 
force routing through a specific interface.

There are multiple, legitimate, uses of an interface index.
Initially these 2 methods were kept private because of some security 
concerns. After consideration and maturation of  the NetworkInterface 
class, these concerns were deemed not justified any longer.


begin:vcard
fn:Jean-Christophe Collet
n:Collet;Jean-Christophe
org:Sun Microsystems;Java Security and Networking
adr:;;2 rue de Jargonnant;Geneva;GE;1702;Switzerland
email;internet:[EMAIL PROTECTED]
x-mozilla-html:FALSE
version:2.1
end:vcard

hg: jdk7/jsn/jdk: 6692802: HttpCookie needs to support HttpOnly attribute

[jean-christophe . collet]

Changeset: eb342082e2b6
Author:jccollet
Date:  2008-09-04 15:26 +0200
URL:   http://hg.openjdk.java.net/jdk7/jsn/jdk/rev/eb342082e2b6

6692802: HttpCookie needs to support HttpOnly attribute
Summary: Added HttpOnly tag support to HttpCookie class.
Reviewed-by: chegar, michaelm

! src/share/classes/java/net/HttpCookie.java
! test/java/net/CookieHandler/TestHttpCookie.java

Re: CFV: Project sponsorship: SCTP

[Jean-Christophe Collet]

Vote: yes

Michael McMahon wrote:

Greetings, voting members of the Networking Group!

Question: Should the Networking Group sponsor the "SCTP" Project[1]?

Please cast your vote by replying, publicly, to this message with either

Vote: yes

or

Vote: no

as the first line of the message body.

You may, at your option, indicate the reason for your decision on
subsequent lines.

Votes must be cast in the open; votes sent as private replies will not be
counted.

The sponsorship decision will be made by a simple majority vote of the
Group's Members.  Votes are due by midnight UTC next November 4, 2008.

As an optimization, if an absolute majority of the Group's Members votes
one way or the other prior to that time then the decision may be rendered
earlier.  Once a decision has been made the votes will be tallied and
reported to this list and also to [EMAIL PROTECTED]

Only Members of the Networking Group are eligible to vote on this
decision.  The current Members are:

   Alan Bateman
   Christopher Hegarty
   Michael McMahon
   Jean-Christophe Collet

Thanks,
Michael

[1] 
http://mail.openjdk.java.net/pipermail/net-dev/2008-October/000427.html

hg: jdk7/tl/jdk: 6790677: java.net.HttpCookie.parse(String) should ignore unrecognized attributes, RFC2965

[jean-christophe . collet]

Changeset: 53d9259661c3
Author:jccollet
Date:  2009-01-27 11:36 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/53d9259661c3

6790677: java.net.HttpCookie.parse(String) should ignore unrecognized 
attributes, RFC2965
Summary: Changed code not to throw an exception on unknown attributes
Reviewed-by: chegar

! src/share/classes/java/net/HttpCookie.java
! test/java/net/CookieHandler/TestHttpCookie.java

hg: jdk7/tl/jdk: 6791927: Wrong Locale in HttpCookie::expiryDate2DeltaSeconds

[jean-christophe . collet]

Changeset: 6c5d04d1eff4
Author:jccollet
Date:  2009-02-02 16:50 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/6c5d04d1eff4

6791927: Wrong Locale in HttpCookie::expiryDate2DeltaSeconds
Summary: Force Locale.US when parsing the cookie expiration date.
Reviewed-by: chegar

! src/share/classes/java/net/HttpCookie.java
+ test/java/net/CookieHandler/B6791927.java

hg: jdk7/tl/jdk: 6585546: Please update API doc for java.net.CookieManager

[jean-christophe . collet]

Changeset: 61ee91f965ac
Author:jccollet
Date:  2009-02-04 14:15 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/61ee91f965ac

6585546: Please update API doc for java.net.CookieManager
Summary: Trivial doc updates
Reviewed-by: chegar

! src/share/classes/java/net/CookieManager.java

[Fwd: Re: [Fwd: CR 6800805 - code review]]

[Jean-Christophe Collet]

--- Begin Message ---

Looks good to me.
I assume it will have a noreg-hard tag?

Christopher Hegarty - Sun Microsystems Ireland wrote:
Even though the code reviews are going external now, I still expect 
that we are the only ones going to review them. Any takers?


-Chris.

 Original Message 
Subject: CR 6800805 - code review
Date: Fri, 13 Feb 2009 16:19:46 +
From: Christopher Hegarty - Sun Microsystems Ireland 


Reply-To: christopher.hega...@sun.com
To: OpenJDK Network Dev list 

I need to get a code review for
  6800805: java.net.NetworkInterface.getNetworkInterfaces() does not
list IPv6 network interfaces correctly.

http://cr.openjdk.java.net/~chegar/6800805/webrev.00/

There is a change in behaviour between JDK5 and JDK6 in how virtual
interfaces addresses are reported. This is as a result of the change for
CR 6386190. This change uses an ioctl to determine if the parent
interface is accessible from the current context. The problem is that on
Solaris if the interface being queried has only IPv6 addresses assigned
to it then the ioctl needs to be called with an IPv6 socket. This is
currently not the case.

To fix this bug we need to try with an IPv6 socket before determining
that the parent is not accessible, similar to what we done for CR 
6628569.


Thanks,
-Chris.




--- End Message ---
begin:vcard
fn:Jean-Christophe Collet
n:Collet;Jean-Christophe
org:Sun Microsystems
adr:;;2 rue de Jargonnant;Geneva;;1207;Switzerland
email;internet:jean-christophe.col...@sun.com
title:Staff Engineer
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Request for Review: 6737323

[Jean-Christophe Collet]

Christopher Hegarty - Sun Microsystems Ireland wrote:

Hi Michael, Jessie,

Please take a few moments to review this trivial change:

   6737323: Typo in javadoc for SocketPermission
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6737323
   http://cr.openjdk.java.net/~chegar/6737323/webrev.00/webrev/

Very trivial changes - remove redundant line from javadoc. The 
permission is not incorrect, but may add confusion since the 
description following it does not mention it. I think the original 
intent was to build on the permission previously mentioned in the 
class description, but it still seems unnecessary.


-Chris.


Looks good to me.

Re: Review request for 6817246

[Jean-Christophe Collet]

Alan Bateman wrote:

Mandy Chung wrote:
6817246: Redundant call to set InetAddressCachePolicy to FOREVER if 
not set during initialization


Webrev at:
   http://cr.openjdk.java.net/~mchung/6817246/webrev.00/

System.setSecurityManager0 calls 
InetAddressCachePolicy.setIfNotSet(InetAddressCachePolicy.FOREVER) to 
set the cache policy to FOREVER if not set by the command-line 
property.  In fact, this call is redundant since the default cache 
policy is already set to FOREVER by the static initializer of the 
InetAddressCachePolicy class.


This fix is to remove the InetAddressCachePolicy.setIfNotSet method 
call from System.setSecurityManager0.


I ran the io and net jtreg tests.

Thanks
Mandy
This looks okay to me and I don't see any side effects to have the 
caching policy initialized a bit later.


-Alan.

Looks good to me too.

Code review for 6726695 (HTTP 100-continue issue)

[Jean-Christophe Collet]

I've put out a webrev for the proposed fix to 6726695 (the Expect: 
100-Continue problem)
There is an explanation of the issue and an example of code usage on my 
blog at http://blogs.sun.com/jcc/


http://cr.openjdk.java.net/~jccollet/6726695/webrev.00/

Re: Review request for 6819122

[Jean-Christophe Collet]

The changes look good to me.

Mandy Chung wrote:
6819122: DefaultProxySelector should lazily initialize the Pattern 
object and the NonProxyInfo objects


Webrev at:
 http://cr.openjdk.java.net/~mchung/6819122/webrev.00/

Details:
1. The Pattern object is only used for checking a IPv6 loopback 
address.   The current fix is to create a Pattern object for every 
check instead of caching it.   The other alternative is to cache the 
Pattern object at the first time it is needed.   Do you believe 
typically applications will hit this path frequently such that caching 
the Pattern object is needed?
 
2. Move the static NonProxyInfo fields into the NonProxyInfo classes 
so that they will only be initialized when it is needed.


Thanks
Mandy


begin:vcard
fn:Jean-Christophe Collet
n:Collet;Jean-Christophe
org:Sun Microsystems
adr:;;2 rue de Jargonnant;Geneva;;1207;Switzerland
email;internet:jean-christophe.col...@sun.com
title:Staff Engineer
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Questions from Apple

[Jean-Christophe Collet]

Misha Bykov wrote:


Apple engineers have some questions about proxy settings in JDK (not 
deploy or applets).


1) Does the JDK support Automatic proxy configuration (WPAD) ? They know
that applets and Webstart do, but they are interested in standalone 
java applications.




No.


2) Does the JDK support PAC files ? (Again, they know that applets and
webstart does, through the browser settings).


No this requires a javascript interpreter which we don't have in 
standalone JDK.




3) On windows, does the JDK (not deploy) actually use the system proxy
settings as defined in control panel ?

Actually, on Windows, it does not seem to, but they wanted to be sure. 
They are aware that we can specify java properties on the command line 
to use the proxy as documented here:


http://java.sun.com/javase/6/docs/technotes/guides/net/properties.html


Actually that same document specifies that setting the system property 
java.net.useSystemProxies to true will do the trick.
However in Windows the way we recover the settings might be a bit 
antiquated (we use registry entries used by I.E.).

I think there is a bug/RFE filed on tis.



In the above list there is no setting for a PAC file or auto-config.

Is there a contact in the networking team that Apple could get in touch
with?



That would be me, or the rest of the networking team 
(net-dev@openjdk.java.net)

Thanks,
Misha





begin:vcard
fn:Jean-Christophe Collet
n:Collet;Jean-Christophe
org:Sun Microsystems
adr:;;2 rue de Jargonnant;Geneva;;1207;Switzerland
email;internet:jean-christophe.col...@sun.com
title:Staff Engineer
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Questions from Apple

[Jean-Christophe Collet]

Roman Kennke wrote:

Hi,



  

2) Does the JDK support PAC files ? (Again, they know that applets and
webstart does, through the browser settings).
  
No this requires a javascript interpreter which we don't have in 
standalone JDK.



No? Isn't javax.script an API for scripting, and actually has a
JavaScript interpreter underneath?

/Roman

  

Ok, I should have been a bit more specific I guess.
We didn't have a scripting engine when we introduced the ProxySelector 
API back in 1.5
Even now it's still more complicated than that, since PAC files relies 
also on a number of library functions like shExpMatch(), isInNet(), 
dnsResolve() etc... which are not part of the engine per-se.
In any cases this would be a significant amount of work, but the good 
news is that the ProxySelector framework has been designed to allow for 
plugins.


begin:vcard
fn:Jean-Christophe Collet
n:Collet;Jean-Christophe
org:Sun Microsystems
adr:;;2 rue de Jargonnant;Geneva;;1207;Switzerland
email;internet:jean-christophe.col...@sun.com
title:Staff Engineer
x-mozilla-html:FALSE
version:2.1
end:vcard

Code review needed for 6349566 (Cookies default domain)

[Jean-Christophe Collet]

This is a very simple fix: Domain of a cookie should default to the host 
name if not specified.


Webrev available at:
http://cr.openjdk.java.net/~jccollet/6349566/webrev.00/

Thanks,

A proposal for an HTTP stream capture mechanism

[Jean-Christophe Collet]

I have been working on adding better logging and debugging tools to the 
HTTP implementation.
A significant chunk of the bugs that we have to investigate are HTTP 
related and being able to monitor the traffic has always been a problem. 
Using a packet sniffer is cumbersome and not always possible.
So I have been putting together a prototype to make that a lot easier. 
The idea is to be able to turn on traffic capture with a system property 
and a simple "rule" file.


The idea is that you can specify a set of capture rules in a file and 
point to that file with the property 'sun.net.http.captureRules'
The rule file has a simple syntax. 1 rule per line. A line starting with 
a '#' is a comment and is ignored. A rule is a comma separated pair 
regular-expression , file pattern.

e.g.:

http://www\.sun\.com , sun%d.log

tells to capture traffic for all URLs containing http://www.sun.com and 
to dump that traffic into a file sunxx.log (the xx is a randomly 
generated number that ensures the created file doesn't overwrite an 
existing one. Of course that '%d' is optional. If the files exist, then 
every capture is appended to it (which might not be a good idea in case 
of multiple parallel threads).


So to capture all the traffic into a single file, the rule would be:

.* , http.log

A usage example becomes:

$ java -Dsun.net.http.captureRules=capture-rules GetURL http://www.sun.com

where 'capture-rules' is the file containing my set of rules.

I have a first prototype of this working but I'm still refining it. I'll 
publish a webrev in a few days.


Comments?

Re: A proposal for an HTTP stream capture mechanism

[Jean-Christophe Collet]

Max (Weijun) Wang wrote:

Hi Jessie

This is very useful.

What kind of info are dumped? Content and/or headers?


What is dumped is the entire stream, byte for byte, captured at the TCP 
socket level. So, includes request, headers and body. Both ways.


This is supposed to complement the traditional logging of events that is 
being extended as well.
So HttpURLConnection is getting some new logging entries as well as an 
optional Formatter for these entries.I attached an example of the 
created log (independent of the capture mentioned earlier).


What the dump now permits is easy comparison between the raw data (from 
the capture rules) and how it was interpreted by HttpURLConnection (from 
the logging).




Can you also capture the connection info? Sometimes I need to know if 
a new connection is created or the old one is reused.


This is now part of the "normal" log.



The \. in sun\.com looks a little cumbersome. I know .hgignore 
supports several different pattern matching mechanisms, probably worth 
a look?


The idea is to use regular expressions to allow for maximum flexibility 
AND simpler implementation.




Thanks
Max


On Apr 23, 2009, at 10:02 PM, Jean-Christophe Collet wrote:

I have been working on adding better logging and debugging tools to 
the HTTP implementation.
A significant chunk of the bugs that we have to investigate are HTTP 
related and being able to monitor the traffic has always been a 
problem. Using a packet sniffer is cumbersome and not always possible.
So I have been putting together a prototype to make that a lot 
easier. The idea is to be able to turn on traffic capture with a 
system property and a simple "rule" file.


The idea is that you can specify a set of capture rules in a file and 
point to that file with the property 'sun.net.http.captureRules'
The rule file has a simple syntax. 1 rule per line. A line starting 
with a '#' is a comment and is ignored. A rule is a comma separated 
pair regular-expression , file pattern.

e.g.:

http://www\.sun\.com , sun%d.log

tells to capture traffic for all URLs containing http://www.sun.com 
and to dump that traffic into a file sunxx.log (the xx is a 
randomly generated number that ensures the created file doesn't 
overwrite an existing one. Of course that '%d' is optional. If the 
files exist, then every capture is appended to it (which might not be 
a good idea in case of multiple parallel threads).


So to capture all the traffic into a single file, the rule would be:

.* , http.log

A usage example becomes:

$ java -Dsun.net.http.captureRules=capture-rules GetURL 
http://www.sun.com


where 'capture-rules' is the file containing my set of rules.

I have a first prototype of this working but I'm still refining it. 
I'll publish a webrev in a few days.


Comments?







HTTP: ProxySelector Request for http://www.sun.com/
HTTP: Proxy used: DIRECT
HTTP: CookieHandler request for http://www.sun.com/
HTTP: Cookies from handler:
HTTP:   GET / HTTP/1.1
User-Agent: Java/1.7.0-internal
Host: www.sun.com
Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
Connection: keep-alive
HTTP:   HTTP/1.1 200 OK
Server: Sun-Java-System-Web-Server/7.0
Date: Thu, 23 Apr 2009 12:39:40 GMT
P3p: policyref="http://www.sun.com/p3p/Sun_P3P_Policy.xml";, CP="CAO DSP 
COR CUR ADMa DEVa TAIa PSAa PSDa CONi TELi OUR  SAMi PUBi IND PHY ONL PUR COM 
NAV INT DEM CNT STA POL PRE GOV"
Cache-control: public
Proxy-agent: Sun-Java-System-Web-Server/7.0
X-powered-by: Servlet/2.4
X-powered-by: JSP/2.0
Set-cookie: JSESSIONID=d2fd0a01da80e724dd7302199f517; Path=/
Content-type: text/html;charset=UTF-8
Via: 1.1 https-www
Set-cookie: JROUTE=1p6hcsERNu5+EQvn; Path=/
Transfer-encoding: chunked
HTTP: ProxySelector Request for http://www.sun.com/products
HTTP: Proxy used: DIRECT
HTTP: CookieHandler request for http://www.sun.com/products
HTTP: Cookies from handler:
ROUTE=1p6hcsERNu5+EQv
SESSIONID=d2fd0a01da80e724dd7302199f51
HTTP:   GET /products HTTP/1.1
User-Agent: Java/1.7.0-internal
Host: www.sun.com
Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
Connection: keep-alive
Cookie: JROUTE=1p6hcsERNu5+EQvn; 
JSESSIONID=d2fd0a01da80e724dd7302199f517
HTTP:   HTTP/1.1 301 Moved Permanently
Server: Sun-Java-System-Web-Server/7.0
Date: Thu, 23 Apr 2009 12:39:41 GMT
P3p: policyref="http://www.sun.com/p3p/Sun_P3P_Policy.xml";, CP="CAO DSP 
COR CUR ADMa DEVa TAIa PSAa PSDa CONi TELi OUR  SAMi PUBi IND PHY ONL PUR COM 
NAV INT DEM CNT STA POL PRE GOV"
Location: /products/index.jsp
Content-length: 0
HTTP: Redirected from http://www.sun.com/products to 
http://www.sun.com/products/index.jsp
HTTP: ProxySelec

Re: Using ProxySelector to allow users to configure the proxy

[Jean-Christophe Collet]

Paulo Levi wrote:
Anyone read the first message at all? The problem is that other proxy 
requests are effectively ignored after a DIRECT connection even if it 
fails.


What i am trying right now is implementing ProxySelector so that a 
"manual" proxy configuration is added at the end of the
public List select(final URI uri) 
return list, and only requesting configuration from the user if
public void connectFailed(final URI uri, final SocketAddress sa, final 
IOException ioe)

function is called.

Obviously this won't work how the current setup is done. What is so 
special about direct connection requests that they must break the 
proxy chain of responsibility?
That's because DIRECT means "do not use a proxy" and 
ProxySelector.connectFailed() is only called when a connection to a 
proxy failed so that it can, potentially, be removed from the list of 
available proxies.
Since no proxy connection was attempted, the callback is not triggered 
even if the connection failed.


begin:vcard
fn:Jean-Christophe Collet
n:Collet;Jean-Christophe
org:Sun Microsystems
adr:;;2 rue de Jargonnant;Geneva;;1207;Switzerland
email;internet:jean-christophe.col...@sun.com
title:Staff Engineer
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Using ProxySelector to allow users to configure the proxy

[Jean-Christophe Collet]

Paulo Levi wrote:
But it's completely unintuitive! I can't know if i need to bother the 
user about proxy configuration before trying with no proxy, but trying 
to use that disallows the use of any other.


Trying to wrap Proxy.NO_PROXY to circumvent that, even if technically 
possible, just makes the code fail in the next line.


if (p.type() != Proxy.Type.SOCKS)
throw new SocketException("Unknown proxy type : " + p.type());

I'm doing this because it's the natural way to create the behavior i 
want, only bugging the user for the proxy if needed.
If you have another idea that doesn't involve bogus requests to 
google.com  or other "know assumed forever" sites 
i'd like to hear it.


The most likely way to tell that you're behind a firewall is to catch 
NoRouteToHostException which is thrown when trying to connect to an 
address that can't be reached. As it is hinted in its javadoc: 
http://java.sun.com/javase/6/docs/api/java/net/NoRouteToHostException.html
However, firewalls vary in nature, so there is no unique way to detect 
them. For instance in some cases you won't even be able to resolve the 
URL hostname because the internal DNS won't resolve external addresses.
There is a reason web browsers don't auto-detect you're behind a 
firewall


Your best bet, I think, is to attempt a direct connection while catching 
both NoRouteToHostException and UnknownHostException. If either of these 
happen, assuming you're certain of your host name, chances are you are 
behind a firewall.
begin:vcard
fn:Jean-Christophe Collet
n:Collet;Jean-Christophe
org:Sun Microsystems
adr:;;2 rue de Jargonnant;Geneva;;1207;Switzerland
email;internet:jean-christophe.col...@sun.com
title:Staff Engineer
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: Request for Review 6837982

[Jean-Christophe Collet]

I approve these changes.

Christopher Hegarty - Sun Microsystems Ireland wrote:

Hi Jessie, Michael, Alan,

I would like to try and get this simple change into b59 through the 
build integration slot, and therefore make M3. The change is simply to 
the docs Makefile so that the SCTP API docs are generated.


CR 6837982:
  SCTP API docs not being generated.

Webrev:
  http://cr.openjdk.java.net/~chegar/6837982/webrev.00/webrev/

Thanks,
-Chris.


begin:vcard
fn:Jean-Christophe Collet
n:Collet;Jean-Christophe
org:Sun Microsystems
adr:;;2 rue de Jargonnant;Geneva;;1207;Switzerland
email;internet:jean-christophe.col...@sun.com
title:Staff Engineer
x-mozilla-html:FALSE
version:2.1
end:vcard

hg: jdk7/tl/jdk: 6349566: java.net.CookieManager doesn't set default domain

[jean-christophe . collet]

Changeset: 206d73d299d4
Author:jccollet
Date:  2009-05-25 22:27 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/206d73d299d4

6349566: java.net.CookieManager doesn't set default domain
Summary: Enforce default domain in CookieManager
Reviewed-by: michaelm

! src/share/classes/java/net/CookieManager.java
! test/java/net/CookieHandler/CookieManagerTest.java

hg: jdk7/tl/jdk: 6726695: HttpURLConnection shoul support 'Expect: 100-contimue' headers for PUT

[jean-christophe . collet]

Changeset: 045aeb76b0ff
Author:jccollet
Date:  2009-05-26 16:03 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/045aeb76b0ff

6726695: HttpURLConnection shoul support 'Expect: 100-contimue' headers for PUT
Summary: Added code triggered when 'Expect: 100-continue' header has been added
Reviewed-by: chegar

! src/share/classes/sun/net/www/http/HttpClient.java
! src/share/classes/sun/net/www/http/KeepAliveStreamCleaner.java
! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java
+ test/sun/net/www/http/HttpClient/B6726695.java

hg: jdk7/tl/jdk: 6852108: Remove Preferences dependance from SocksSocketImpl

[jean-christophe . collet]

Changeset: ed38f9e6ad9a
Author:jccollet
Date:  2009-06-19 14:12 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/ed38f9e6ad9a

6852108: Remove Preferences dependance from SocksSocketImpl
Summary: Removed Preferences API use and fixed a few findbugs gotchas
Reviewed-by: alanb

! src/share/classes/java/net/SocksSocketImpl.java

Code review for 6811297: Add more logging to HTTP protocol handler

[Jean-Christophe Collet]

Here is proposal to add more debugging tool for the HTTP protocol handler.
This adds more logging, a formatter for the logs and a traffic capture tool.
These changes have no implications on normal operation, they just add 
features that can be turned on when investigating bugs.


A webrev is available at 
http://cr.openjdk.java.net/~jccollet/6811297/webrev.00/


Thanks,

hg: jdk7/tl/jdk: 6811297: Add more logging to HTTP protocol handler

[jean-christophe . collet]

Changeset: 70c0a927e21a
Author:jccollet
Date:  2009-06-25 18:56 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/70c0a927e21a

6811297: Add more logging to HTTP protocol handler
Summary: Added extra logging to HttpURLConnection and HttpClient. Added a 
capture tool.
Reviewed-by: chegar

! make/sun/net/FILES_java.gmk
+ src/share/classes/sun/net/www/http/HttpCapture.java
+ src/share/classes/sun/net/www/http/HttpCaptureInputStream.java
+ src/share/classes/sun/net/www/http/HttpCaptureOutputStream.java
! src/share/classes/sun/net/www/http/HttpClient.java
+ src/share/classes/sun/net/www/protocol/http/HttpLogFormatter.java
! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java

hg: jdk7/tl/jdk: 6855297: Windows build breaks after 6811297

[jean-christophe . collet]

Changeset: 0b6571d4b4b5
Author:jccollet
Date:  2009-06-26 16:50 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/0b6571d4b4b5

6855297: Windows build breaks after 6811297
Summary: re-introduced the mistakenly taken out authObj member
Reviewed-by: chegar

! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java

Re: hg: jdk7/tl/jdk: 6811297: Add more logging to HTTP protocol handler

[Jean-Christophe Collet]

Yes, it would be nice and there are actually already 2 RFEs covering this:
4640805: Protocol and content handlers SPI
6249864: make it easier to write custom url handlers

However this is no small task, specially considering the compatibility 
and security constraints.

So far resources (or lack of thereof) have prevented us of working on it.

Jim Andreou wrote:

Hi,

A quick note: Wouldn't it be nice if clients could add URL handling 
interceptors and monitor incoming/outgoing data themselves? For any 
URL protocol they would be interested into, not http in particular. 
Recently I looked into the existing URL protocol handling architecture 
(e.g. [1], which promises a new era for protocol handlers, but it 
seems that the existing support is cumbersome and  restricting - 
sorry, I can't elaborate right now) but it seems to make any such 
scenario impossible. 


Any comments?

Thanks,

Dimitris Andreou

[1] http://java.sun.com/developer/onlineTraining/protocolhandlers/

2009/6/25 >


Changeset: 70c0a927e21a
Author:jccollet
Date:  2009-06-25 18:56 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/70c0a927e21a

6811297: Add more logging to HTTP protocol handler
Summary: Added extra logging to HttpURLConnection and HttpClient.
Added a capture tool.
Reviewed-by: chegar

! make/sun/net/FILES_java.gmk
+ src/share/classes/sun/net/www/http/HttpCapture.java
+ src/share/classes/sun/net/www/http/HttpCaptureInputStream.java
+ src/share/classes/sun/net/www/http/HttpCaptureOutputStream.java
! src/share/classes/sun/net/www/http/HttpClient.java
+ src/share/classes/sun/net/www/protocol/http/HttpLogFormatter.java
! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java

hg: jdk7/tl/jdk: 6856856: NPE in HTTP protocol handler logging

[jean-christophe . collet]

Changeset: fa488e4ff685
Author:jccollet
Date:  2009-07-06 15:13 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/fa488e4ff685

6856856: NPE in HTTP protocol handler logging
Summary: Fixed the NPE and Moved the java.util.logging dependency to a single 
class and used reflection to make it a soft one.
Reviewed-by: chegar

! src/share/classes/sun/net/www/http/HttpCapture.java
! src/share/classes/sun/net/www/http/HttpClient.java
! src/share/classes/sun/net/www/protocol/http/HttpLogFormatter.java
! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java

Code review needed for 6737819

[Jean-Christophe Collet]

Here are the changes for 6737819.
This allows for the previously hard-coded, always in, values for 
http.nonProxyHosts and ftp.nonProxyHosts to be overridden when necessary.
I.E. before it was never possible to reach localhost (and its 
variations) via a proxy (hard coded exemption). These changes keep that 
behavior by default, but make it possible to change it.


http://cr.openjdk.java.net/~jccollet/6737819/webrev.00/

hg: jdk7/tl/jdk: 6737819: sun.misc.net.DefaultProxySelector doesn't use proxy setting to localhost

[jean-christophe . collet]

Changeset: ee68feef41d2
Author:jccollet
Date:  2009-09-18 10:51 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/ee68feef41d2

6737819: sun.misc.net.DefaultProxySelector doesn't use proxy setting to 
localhost
Summary: Move default nonProxyHosts from hardcoded to property default value
Reviewed-by: chegar

! src/share/classes/java/net/doc-files/net-properties.html
! src/share/classes/sun/net/spi/DefaultProxySelector.java
! src/share/lib/net.properties
+ test/java/net/ProxySelector/B6737819.java

Code review needed for 6873543

[Jean-Christophe Collet]

Here are my proposed changes to fix 6873543 (httpOnly cookies).
Also took the opportunity to clarify the javadoc of a few methods.

http://cr.openjdk.java.net/~jccollet/6873543/webrev.00/

Re: Request for Review 6882384

[Jean-Christophe Collet]

Christopher Hegarty - Sun Microsystems Ireland wrote:


6882384: Update http protocol handler to use PlatformLogger

Webrev:
  http://cr.openjdk.java.net/~chegar/6882384/webrev.0/webrev/

Change http logging to use the PlatformLogger. We can simply remove 
the reflective calls to the logging API from HttpCapture as the 
PlatformLogger will take care of that for us.


-Chris.

Looks good to me.

<>

Code review needed for 6519647

[Jean-Christophe Collet]

These changes provide 2 things:
- a minor bug fix for 6519647  (FTP PUT operation through HTTP proxy)
- an overhaul of the FTP code and laying the bases of a future public 
FTP Client API.

That's why the code is separated between sun.net.ftp and sun.net.ftp.iml
Ultimately, what is in sun.net.ftp will become public either in java.net 
or java.net.ftp


http://cr.openjdk.java.net/~jccollet/6519647/webrev.0/

hg: jdk7/tl/jdk: 6873543: CookieManager doesn't enforce httpOnly

[jean-christophe . collet]

Changeset: 5ab37d9d9260
Author:jccollet
Date:  2009-10-21 13:42 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/5ab37d9d9260

6873543: CookieManager doesn't enforce httpOnly
Summary: Adds check for httpOnly tag and clarifies javadoc
Reviewed-by: chegar

! src/share/classes/java/net/CookieHandler.java
! src/share/classes/java/net/CookieManager.java
! test/java/net/CookieHandler/B6644726.java

hg: jdk7/tl/jdk: 6893702: Overhaul of Ftp Client internal code

[jean-christophe . collet]

Changeset: 75c453fa1aa1
Author:jccollet
Date:  2009-10-21 16:28 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/75c453fa1aa1

6893702: Overhaul of Ftp Client internal code
Summary: Major reorg of internal FTP client code
Reviewed-by: chegar

! make/sun/net/FILES_java.gmk
! src/share/classes/sun/net/ftp/FtpClient.java
+ src/share/classes/sun/net/ftp/FtpClientProvider.java
+ src/share/classes/sun/net/ftp/FtpDirEntry.java
+ src/share/classes/sun/net/ftp/FtpDirParser.java
! src/share/classes/sun/net/ftp/FtpLoginException.java
! src/share/classes/sun/net/ftp/FtpProtocolException.java
+ src/share/classes/sun/net/ftp/FtpReplyCode.java
+ src/share/classes/sun/net/ftp/impl/DefaultFtpClientProvider.java
+ src/share/classes/sun/net/ftp/impl/FtpClient.java
! src/share/classes/sun/net/www/protocol/ftp/FtpURLConnection.java

Re: Need reviewer for 6894633: NetHooks hould not require provider to be present (sol)

[Jean-Christophe Collet]

Christopher Hegarty - Sun Microsystems Ireland wrote:

The changes look fine.



Same here. Looks fine.


-Chris.

On 23/10/2009 14:57, Alan Bateman wrote:


I need a reviewer for a small fix to sun.net.NetHooks. On Solaris, 
the code incorrectly assumes that the sdp provider is always present. 
The loadProvider method should return null if not present. The diffs 
are attached.


Thanks,
Alan.


--- a/src/solaris/classes/sun/net/NetHooks.java
+++ b/src/solaris/classes/sun/net/NetHooks.java
@@ -81,7 +81,7 @@ public final class NetHooks {
try {
c = (Class)Class.forName(cn, true, 
null);

} catch (ClassNotFoundException x) {
-throw new AssertionError(x);
+return null;
}
try {
return c.newInstance();

Need code review for 6901170

[Jean-Christophe Collet]

This is a simple fix for a cookie parsing issue.

http://cr.openjdk.java.net/~jccollet/6901170/webrev.0/

Thanks,

<>

hg: jdk7/tl/jdk: 6901170: HttpCookie parsing of version and max-age mis-handled

[jean-christophe . collet]

Changeset: ca026eb5cf3c
Author:jccollet
Date:  2009-11-20 14:50 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/ca026eb5cf3c

6901170: HttpCookie parsing of version and max-age mis-handled
Summary: Accept single quotes in cookies and better exception handling in 
CookieManager
Reviewed-by: chegar

! src/share/classes/java/net/CookieManager.java
! src/share/classes/java/net/HttpCookie.java
! test/java/net/CookieHandler/TestHttpCookie.java

Re: Fwd: Bug in URLConnection?

[Jean-Christophe Collet]

Paulo Levi wrote:
Just realized you might be confused about the saving of the original 
proxyselector in the test code. To be clear:
What is called and throws the StackOverflowError is the 
UserProxySelector class. I just expected calling 
url.openConnection(Proxy.NO_PROXY); not to call the userproxyselector 
(or any ProxySelector) so it shouldn't recurse so no 
StackOverflowException.


Saving the original proxy selector is just not to affect any other tests.

On Mon, Dec 7, 2009 at 3:43 PM, Paulo Levi > wrote:


Huh?
But it throws StackOverflowError. after installing
UserProxySelector and calling any method. Just to be clear it is
the right class, it is attached.
My java version:
E:\java\bin>java -version
java version "1.6.0_14-ea"
Java(TM) SE Runtime Environment (build 1.6.0_14-ea-b04)
Java HotSpot(TM) Client VM (build 14.0-b13, mixed mode)

Windows xp.


What's happening here is that the ProxySelector() is not called to check 
whether the URLConnection should use a proxy but whether the Socket 
should use a SOCKS proxy.

Hence the endless recursive loop.
If you're going to make network calls from the ProxySelector 
implementation (which is not recommended), make sure you cover your 
bases to avoid that kind of problem.

In your case, having select be:
   @Override
   public List select(URI uri) {
   if (uri.getScheme().equalsIgnorCase("http")) { // Avoid the 
"socket://" endless loop

try {
 //bug here, the java doc say that this will bypass the 
installed

 //proxyselector but it doesn't.
 URL u = new URL("http://www.google.com";);
 URLConnection conn = u.openConnection(Proxy.NO_PROXY);
 conn.connect();
 } catch (Exception ex) {
 
Logger.getLogger(UserProxySelector.class.getName()).log(Level.SEVERE, 
null, ex);

 }
 }
   return Collections.singletonList(Proxy.NO_PROXY);
   }

I'm not sure this should be considered a bug. But it might be worth it 
to add a few notes in ProxySelector javadoc to warn against such problems.

Re: Request for Review: 6909089

[Jean-Christophe Collet]

I approve this change.

On 12/10/09 16:35, Christopher Hegarty -Sun Microsystems Ireland wrote:


CR 6909089: Memory leak occurs by lack of free for read buffer in 
SocketInputStream#read()


Webrev:
   http://cr.openjdk.java.net/~chegar/6909089/webrev.0/webrev/

Bug description says it all.

-Chris.


<>

hg: jdk7/tl/jdk: 6919185: test/closed/sun/net/ftp/FtpTests fails to compile

[jean-christophe . collet]

Changeset: f544825d0976
Author:jccollet
Date:  2010-01-26 11:39 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/f544825d0976

6919185: test/closed/sun/net/ftp/FtpTests fails to compile
Summary: Fixed a couple of regressions in FtpClient and updated the test.
Reviewed-by: chegar

! src/share/classes/sun/net/ftp/impl/FtpClient.java