You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Kevin Zhou <zh...@gmail.com> on 2010/04/20 11:55:05 UTC
Re: svn commit: r935847 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src:
main/java/org/apache/harmony/luni/internal/net/www/protocol/file/ test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/
On 2010-4-20 17:38, zhoukevin@apache.org wrote:
> Author: zhoukevin
> Date: Tue Apr 20 09:38:04 2010
> New Revision: 935847
>
> URL: http://svn.apache.org/viewvc?rev=935847&view=rev
> Log:
> This patch implements the header related functions [getHeaderField(int)/getHeaderFieldKey(int)/getHeaderField(String)/getLastModified()] to prevent Harmony to return null when file URL is handled.
>
> Modified:
> harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/file/FileURLConnection.java
> harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/FileURLConnectionTest.java
>
> Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/file/FileURLConnection.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/file/FileURLConnection.java?rev=935847&r1=935846&r2=935847&view=diff
> ==============================================================================
> --- harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/file/FileURLConnection.java (original)
> +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/file/FileURLConnection.java Tue Apr 20 09:38:04 2010
> @@ -28,6 +28,9 @@ import java.io.InputStream;
> import java.io.PrintStream;
> import java.net.URL;
> import java.net.URLConnection;
> +import java.text.SimpleDateFormat;
> +import java.util.Date;
> +import java.util.LinkedHashMap;
>
> import org.apache.harmony.luni.internal.net.www.MimeTable;
> import org.apache.harmony.luni.util.Util;
> @@ -48,8 +51,12 @@ public class FileURLConnection extends U
>
> private boolean isDir;
>
> + private long lastModified = 0;
> +
> private FilePermission permission;
>
> + private LinkedHashMap<String, String> header;
> +
> /**
> * Creates an instance of<code>FileURLConnection</code> for establishing
> * a connection to the file pointed by this<code>URL<code>
> @@ -63,6 +70,7 @@ public class FileURLConnection extends U
> fileName = ""; //$NON-NLS-1$
> }
> fileName = Util.decode(fileName, false);
> + header = new LinkedHashMap<String, String>();
> }
>
> /**
> @@ -76,15 +84,90 @@ public class FileURLConnection extends U
> @Override
> public void connect() throws IOException {
> File f = new File(fileName);
> - if (f.isDirectory()) {
> - isDir = true;
> + if (isDir = f.isDirectory()) {
> is = getDirectoryListing(f);
> - // use -1 for the contentLength
> } else {
> is = new BufferedInputStream(new FileInputStream(f));
> length = is.available();
> + lastModified = f.lastModified();
> }
> connected = true;
> + // updaetHeader must be after "connected = true", for updateHeader
> + // invokes getContentType which back invokes connect causing
> + // StackOverflow
> + updateHeader();
> + }
> +
> + /**
> + * Function updates the header fields of a file.
> + */
> + private void updateHeader() {
> + String contentType = getContentType();
> + if (contentType != null) {
> + header.put("content-type", contentType); //$NON-NLS-1$
> + }
> + if (length>= 0) {
> + header.put("content-length", Integer.toString(length)); //$NON-NLS-1$
> + }
> + if (lastModified != 0) {
> + SimpleDateFormat dateFormat = new SimpleDateFormat(
> + "EEE, dd MMM yyyy HH:mm:ss 'GMT'"); //$NON-NLS-1$
> + header.put("last-modified", dateFormat.format(new Date( //$NON-NLS-1$
> + lastModified)));
> + }
> + }
> +
> + @Override
> + public String getHeaderField(String key) {
> + try {
> + if (!connected) {
> + connect();
> + }
> + } catch (IOException e) {
> + // Ignored
> + }
> + return header.get(key);
> + }
> +
> + @Override
> + public String getHeaderField(int index) {
> + try {
> + if (!connected) {
> + connect();
> + }
> + } catch (IOException e) {
> + // Ignored
> + }
> + if (index> -1&& index< header.size()) {
> + return header.values().toArray(new String[0])[index];
> + }
> + return null;
> + }
> +
> + @Override
> + public String getHeaderFieldKey(int index) {
> + try {
> + if (!connected) {
> + connect();
> + }
> + } catch (IOException e) {
> + // Ignored
> + }
> + if (index> -1&& index< header.size()) {
> + return header.keySet().toArray(new String[0])[index];
> + }
> + return null;
> + }
> +
> + public long getLastModified() {
> + try {
> + if (!connected) {
> + connect();
> + }
> + } catch (IOException e) {
> + // default is -1
> + }
> + return lastModified;
> }
>
> /**
>
> Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/FileURLConnectionTest.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/FileURLConnectionTest.java?rev=935847&r1=935846&r2=935847&view=diff
> ==============================================================================
> --- harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/FileURLConnectionTest.java (original)
> +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/FileURLConnectionTest.java Tue Apr 20 09:38:04 2010
> @@ -18,6 +18,7 @@ package org.apache.harmony.luni.tests.in
>
> import java.io.IOException;
> import java.net.URL;
> +import java.net.URLConnection;
>
> import junit.framework.TestCase;
>
> @@ -63,4 +64,41 @@ public class FileURLConnectionTest exten
> assertNotNull(conn.getInputStream());
> assertEquals("file",conn.getURL().getProtocol());
> }
> +
> + public void testHeaderFunctions() throws IOException {
> + String resourceName = "org/apache/harmony/luni/tests/"; // folder name
> + URL url = ClassLoader.getSystemClassLoader().getResource(resourceName);
> + FileURLConnection conn = new FileURLConnection(url);
> + assertNotNull(conn.getInputStream());
> + assertEquals(conn.getContentType(), conn.getHeaderField("content-type"));
> +
> + resourceName = "org/apache/harmony/luni/tests/" + "test.rtf";
> + ; // folder name
> + url = ClassLoader.getSystemClassLoader().getResource(resourceName);
> + conn = new FileURLConnection(url);
> + assertNotNull(conn.getInputStream());
> + assertEquals(conn.getContentType(), conn.getHeaderField("content-type"));
> + assertEquals(Integer.toString(conn.getContentLength()), conn
> + .getHeaderField("content-length"));
> + assertEquals(conn.getHeaderField(0), conn
> + .getHeaderField("content-type"));
> + assertEquals(conn.getHeaderField(1), conn
> + .getHeaderField("content-length"));
> + assertEquals(conn.getHeaderField(2), conn
> + .getHeaderField("last-modified"));
> + assertEquals("last-modified", conn.getHeaderFieldKey(2));
> + assertEquals("content-length", conn.getHeaderFieldKey(1));
> + assertEquals("content-type", conn.getHeaderFieldKey(0));
> + }
> +
> + public void testHeader_BoundaryCheck() throws IOException {
> + String resourceName = "org/apache/harmony/luni/tests/";
> + URL url = ClassLoader.getSystemClassLoader().getResource(resourceName);
> + URLConnection urlConnection = url.openConnection();
> + assertNull(urlConnection.getHeaderField(Integer.MIN_VALUE));
> + assertNull(urlConnection.getHeaderField(Integer.MAX_VALUE));
> + assertNull(urlConnection.getHeaderFieldKey(Integer.MIN_VALUE));
> + assertNull(urlConnection.getHeaderFieldKey(Integer.MAX_VALUE));
> + assertNull(urlConnection.getHeaderField(null));
> + }
> }
>
>
>
This patch is provided by Mohanraj Loganathan, Thanks.
Re: svn commit: r935847 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src: main/java/org/apache/harmony/luni/internal/net/www/protocol/file/ test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/
Posted by Mark Hindess <ma...@googlemail.com>.
In message <o2...@mail.gmail.com>,
Kevin Zhou writes:
>
> Thanks, Mark and Sebb
>
> On Tue, Apr 20, 2010 at 6:37 PM, sebb <se...@gmail.com> wrote:
>
> > Also, consider whether the header field could be made final - it looks
> > like it is only set in the constructor.
Kevin,
I'm still not happy about your commit r935873. Your commit message says:
Apply patch for HARMONY-6504: [classlib][luni]Harmony
returns null for header related function when file URL is
handled. This patch implements the header related functions
[getHeaderField(int)/getHeaderFieldKey(int)/getHeaderField(
String)/getLastModified()] to prevent Harmony to return null when file
URL is handled.
and contains the change:
+ private final LinkedHashMap<String, String> header;
but Mohanraj's patch contains the change:
+ private LinkedHashMap<String, String> header;
So, at the very least, your comment should have said that the commit was
a modified version of the patch with an explanation for the modification
either in the commit message or on the JIRA comment associated with the
commit[0]. (You should probably give credit for the change in this
case as well.)
Personally, if a modification isn't required to integrate the patch then
I prefer to see two commits. One with the patch and one with subsequent
improvements.
Regards,
Mark.
[0] See Oliver's commit r935528 of HARMONY-6476 for example.
Re: svn commit: r935847 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src:
main/java/org/apache/harmony/luni/internal/net/www/protocol/file/
test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/
Posted by Kevin Zhou <zh...@gmail.com>.
Thanks, Mark and Sebb
On Tue, Apr 20, 2010 at 6:37 PM, sebb <se...@gmail.com> wrote:
> Also, consider whether the header field could be made final - it looks
> like it is only set in the constructor.
>
> On 20/04/2010, Kevin Zhou <zh...@gmail.com> wrote:
> > OK, thanks!
> >
> > We'll stick to the process now. I have asked Mohan to raise a JIRA and
> > attach the patch with granting license to ASF, will commit it later.
> >
> > On Tue, Apr 20, 2010 at 6:11 PM, Mark Hindess
> > <ma...@googlemail.com>wrote:
> >
> >
> > >
> > > In message <201004201004.o3KA42Ao021185@d12av04.megacenter.de.ibm.com
> >,
> > > Mark Hindess writes:
> > > >
> > > > In message <4B...@gmail.com>, Kevin Zhou writes:
> > > > >
> > > > > On 2010-4-20 17:38, zhoukevin@apache.org wrote:
> > > > > > Author: zhoukevin
> > > > > > Date: Tue Apr 20 09:38:04 2010
> > > > > > New Revision: 935847
> > > > > >
> > > > > > URL: http://svn.apache.org/viewvc?rev=935847&view=rev
> > > > > > Log:
> > > > > > This patch implements the header related functions
> > > > > > [getHeaderField(int)/get
> > > > > HeaderFieldKey(int)/getHeaderField(String)/getLastModified()] to
> > > prevent
> > > > > Harmony to return null when file URL is handled.
> > > > > >
> > > > > > [SNIP]
> > > > >
> > > > > This patch is provided by Mohanraj Loganathan, Thanks.
> > > >
> > > > Then Mohanraj *must* submit it to JIRA and grant license to the ASF
> to
> > > > use it. You *should* not commit it until he does this.
> > > >
> > > > Please back it out and follow the correct process. The process is
> > > > exists for good reason.
> > >
> > > I've back this out in r935857.
> > > -Mark.
> > >
> > >
> > >
> >
> >
> >
> > --
> > Best regards,
> > Yours, Kevin Zhou
> >
>
--
Best regards,
Yours, Kevin Zhou
Re: svn commit: r935847 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src:
main/java/org/apache/harmony/luni/internal/net/www/protocol/file/
test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/
Posted by sebb <se...@gmail.com>.
Also, consider whether the header field could be made final - it looks
like it is only set in the constructor.
On 20/04/2010, Kevin Zhou <zh...@gmail.com> wrote:
> OK, thanks!
>
> We'll stick to the process now. I have asked Mohan to raise a JIRA and
> attach the patch with granting license to ASF, will commit it later.
>
> On Tue, Apr 20, 2010 at 6:11 PM, Mark Hindess
> <ma...@googlemail.com>wrote:
>
>
> >
> > In message <20...@d12av04.megacenter.de.ibm.com>,
> > Mark Hindess writes:
> > >
> > > In message <4B...@gmail.com>, Kevin Zhou writes:
> > > >
> > > > On 2010-4-20 17:38, zhoukevin@apache.org wrote:
> > > > > Author: zhoukevin
> > > > > Date: Tue Apr 20 09:38:04 2010
> > > > > New Revision: 935847
> > > > >
> > > > > URL: http://svn.apache.org/viewvc?rev=935847&view=rev
> > > > > Log:
> > > > > This patch implements the header related functions
> > > > > [getHeaderField(int)/get
> > > > HeaderFieldKey(int)/getHeaderField(String)/getLastModified()] to
> > prevent
> > > > Harmony to return null when file URL is handled.
> > > > >
> > > > > [SNIP]
> > > >
> > > > This patch is provided by Mohanraj Loganathan, Thanks.
> > >
> > > Then Mohanraj *must* submit it to JIRA and grant license to the ASF to
> > > use it. You *should* not commit it until he does this.
> > >
> > > Please back it out and follow the correct process. The process is
> > > exists for good reason.
> >
> > I've back this out in r935857.
> > -Mark.
> >
> >
> >
>
>
>
> --
> Best regards,
> Yours, Kevin Zhou
>
Re: svn commit: r935847 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src:
main/java/org/apache/harmony/luni/internal/net/www/protocol/file/
test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/
Posted by Kevin Zhou <zh...@gmail.com>.
OK, thanks!
We'll stick to the process now. I have asked Mohan to raise a JIRA and
attach the patch with granting license to ASF, will commit it later.
On Tue, Apr 20, 2010 at 6:11 PM, Mark Hindess
<ma...@googlemail.com>wrote:
>
> In message <20...@d12av04.megacenter.de.ibm.com>,
> Mark Hindess writes:
> >
> > In message <4B...@gmail.com>, Kevin Zhou writes:
> > >
> > > On 2010-4-20 17:38, zhoukevin@apache.org wrote:
> > > > Author: zhoukevin
> > > > Date: Tue Apr 20 09:38:04 2010
> > > > New Revision: 935847
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=935847&view=rev
> > > > Log:
> > > > This patch implements the header related functions
> > > > [getHeaderField(int)/get
> > > HeaderFieldKey(int)/getHeaderField(String)/getLastModified()] to
> prevent
> > > Harmony to return null when file URL is handled.
> > > >
> > > > [SNIP]
> > >
> > > This patch is provided by Mohanraj Loganathan, Thanks.
> >
> > Then Mohanraj *must* submit it to JIRA and grant license to the ASF to
> > use it. You *should* not commit it until he does this.
> >
> > Please back it out and follow the correct process. The process is
> > exists for good reason.
>
> I've back this out in r935857.
> -Mark.
>
>
>
--
Best regards,
Yours, Kevin Zhou
Re: svn commit: r935847 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src: main/java/org/apache/harmony/luni/internal/net/www/protocol/file/ test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/
Posted by Mark Hindess <ma...@googlemail.com>.
In message <20...@d12av04.megacenter.de.ibm.com>,
Mark Hindess writes:
>
> In message <4B...@gmail.com>, Kevin Zhou writes:
> >
> > On 2010-4-20 17:38, zhoukevin@apache.org wrote:
> > > Author: zhoukevin
> > > Date: Tue Apr 20 09:38:04 2010
> > > New Revision: 935847
> > >
> > > URL: http://svn.apache.org/viewvc?rev=935847&view=rev
> > > Log:
> > > This patch implements the header related functions
> > > [getHeaderField(int)/get
> > HeaderFieldKey(int)/getHeaderField(String)/getLastModified()] to prevent
> > Harmony to return null when file URL is handled.
> > >
> > > [SNIP]
> >
> > This patch is provided by Mohanraj Loganathan, Thanks.
>
> Then Mohanraj *must* submit it to JIRA and grant license to the ASF to
> use it. You *should* not commit it until he does this.
>
> Please back it out and follow the correct process. The process is
> exists for good reason.
I've back this out in r935857.
-Mark.
Re: svn commit: r935847 - in /harmony/enhanced/java/trunk/classlib/modules/luni/src: main/java/org/apache/harmony/luni/internal/net/www/protocol/file/ test/api/common/org/apache/harmony/luni/tests/internal/net/www/protocol/file/
Posted by Mark Hindess <ma...@googlemail.com>.
In message <4B...@gmail.com>, Kevin Zhou writes:
>
> On 2010-4-20 17:38, zhoukevin@apache.org wrote:
> > Author: zhoukevin
> > Date: Tue Apr 20 09:38:04 2010
> > New Revision: 935847
> >
> > URL: http://svn.apache.org/viewvc?rev=935847&view=rev
> > Log:
> > This patch implements the header related functions [getHeaderField(int)/get
> HeaderFieldKey(int)/getHeaderField(String)/getLastModified()] to prevent Harm
> ony to return null when file URL is handled.
> >
> > [SNIP]
>
> This patch is provided by Mohanraj Loganathan, Thanks.
Then Mohanraj *must* submit it to JIRA and grant license to the ASF to
use it. You *should* not commit it until he does this.
Please back it out and follow the correct process. The process is
exists for good reason.
Regards,
Mark.