You are viewing a plain text version of this content. The canonical link for it is here.
Posted to slide-dev@jakarta.apache.org by Michael Christoff <Mi...@camh.net> on 2007/02/08 23:46:15 UTC
Fixes for bugs in WebdavResource putMethod and getMethod
Hi all.
I just joined the list and didn't realize bug reports get sent here directly. So I figured I'd send my complete email directly to the list as well as archiving it in bugzilla.
This regards some new bug fixes I coded for the Slide client API. The bug fixes apply to:
org.apache.webdav.lib.WebdavResource.getMethod(String path, File file), and
org.apache.webdav.lib.WebdavResource.putMethod(String path, File file)
I had previously submitted a patch for putMethod(String, File) and had it accepted (see Slide 2.1 bug 40835, http://issues.apache.org/bugzilla/show_bug.cgi?id=40835 )--I was told it would appear in the next release of the API. However I see there is still a problem with this method even with the fix--and it turns out that the problem also occurs in getMethod(String, File).
/////////// ** PUTMETHOD BUG **
Below is the orginal code for putMethod(String, File) :
///////////////////////
1: public boolean putMethod(String path, File file)
2: throws HttpException, IOException {
3:
4: setClient();
5: PutMethod method = new PutMethod(URIUtil.encodePathQuery(path));
6: generateIfHeader(method);
7: if (getGetContentType() != null && !getGetContentType().equals(""))
8: method.setRequestHeader("Content-Type", getGetContentType());
9: long fileLength = file.length();
10: method.setRequestContentLength(fileLength <= Integer.MAX_VALUE
12: ? (int) fileLength
13: : PutMethod.CONTENT_LENGTH_CHUNKED);
14: method.setRequestBody(new FileInputStream(file));
15: generateTransactionHeader(method);
16: int statusCode = client.executeMethod(method);
17:
18: setStatusCode(statusCode);
19: return (statusCode >= 200 && statusCode < 300) ? true : false;
20: }
///////////////////////
As you can see, the FileInputStream created in line 14 is never closed. While this error won't manifest itself in a noticeable way on *nix systems, on Windows a file opened for writing is locked and hence applications on the client side will not be able to edit it from therein (one would get a "File is in use" message).
The original solution was to replace line 14 with following code:
14 : FileInputStream fis = new FileInputStream(file);
14.1: method.setRequestBody(fis);
and then insert the following code after line 18:
18.1: fis.close();
However, the problem that still remains is this: what happens if an exception is thrown between lines 15 and 18 (inclusive)? Specifically, in line 16, the executeMethod of HttpClient may throw an IOException or HttpException. If this occurs then the FileInputStream will still not be closed. The solution to this is below:
/////////// ** PUTMETHOD FIX **
///////////////////////
1: public boolean putMethod(String path, File file)
2: throws HttpException, IOException {
3:
4: FileInputStream fis = null;
5: try {
6: setClient();
7: PutMethod method = new PutMethod(URIUtil.encodePathQuery(path));
8: generateIfHeader(method);
9: if (getGetContentType() != null && !getGetContentType().equals(""))
10: method.setRequestHeader("Content-Type", getGetContentType());
11: long fileLength = file.length();
12: method.setRequestContentLength(fileLength <= Integer.MAX_VALUE
13: ? (int) fileLength
14: : PutMethod.CONTENT_LENGTH_CHUNKED);
15: fis = new FileInputStream(file);
16: method.setRequestBody(fis);
17: generateTransactionHeader(method);
18: int statusCode = client.executeMethod(method);
19:
20: setStatusCode(statusCode);
21: return (statusCode >= 200 && statusCode < 300) ? true : false;
22:
23: } finally {
24: if(fis != null)
25: fis.close();
26: }
27: }
///////////////////////
By enclosing the method's code in a try block, we can ensure fis is always closed, even if an exception occurs after it is instantiated.
-------------------------
/////////// ** GETMETHOD BUG **
The identical problem exists in putMethod(String, File) of the same class. The original code is below:
///////////////////////
1: public boolean getMethod(String path, File file)
2: throws HttpException, IOException {
3:
4: setClient();
5: GetMethod method = new GetMethod(URIUtil.encodePathQuery(path));
6:
7: generateTransactionHeader(method);
8: int statusCode = client.executeMethod(method);
9:
10: setStatusCode(statusCode);
11:
12: // get the file only if status is any kind of OK
13: if (statusCode >= 200 && statusCode < 300) {
14:
15: // Do a simple little loop to read the response back into the passed
16: // file parameter.
17: InputStream inStream = method.getResponseBodyAsStream();
18:
19: FileOutputStream fos = new FileOutputStream(file);
20: byte buffer[] = new byte[65535];
21: int bytesRead;
22: while ((bytesRead = inStream.read(buffer)) >= 0) {
23: fos.write(buffer, 0, bytesRead);
24: }
25: inStream.close();
26: fos.close();
27:
28: return true;
29:
30: } else {
31: return false;
32: }
33: }
If an exception is thrown between lines 20 and 24 (inclusive) the input and output streams inStream and fos will not be closed.
/////////// ** GETMETHOD FIX **
The solution is the same as it was for putMethod(String, File) :
///////////////////////
1: public boolean getMethod(String path, File file)
2: throws HttpException, IOException {
3:
4: setClient();
5: GetMethod method = new GetMethod(URIUtil.encodePathQuery(path));
6:
7: generateTransactionHeader(method);
8: int statusCode = client.executeMethod(method);
9:
10: setStatusCode(statusCode);
11:
12: // get the file only if status is any kind of OK
13: if (statusCode >= 200 && statusCode < 300) {
14:
14.1: FileOutputStream fos = null;
14.2: InputStream inStream = null;
14.3: try {
15: // Do a simple little loop to read the response back into the passed
16: // file parameter.
17*: inStream = method.getResponseBodyAsStream();
18:
19*: fos = new FileOutputStream(file);
20: byte buffer[] = new byte[65535];
21: int bytesRead;
22: while ((bytesRead = inStream.read(buffer)) >= 0) {
23: fos.write(buffer, 0, bytesRead);
24: }
-25-
-26-
27:
28: return true;
28.1:
28.2: } finally {
28.3: if(fos != null)
28.4: fos.close();
28.5: if(inStream != null)
28.6: inStream.close();
28.7: }
29:
30: } else {
31: return false;
32: }
33: }
///////////////////////
Lines 14.1-14.3 and 28.1-28.7 are added code, lines 25 and 26 have been deleted from the original source, and lines 17 and 19 have been updated (we now declare inStream and fos outside the try block in lines 14.1-14.3).
Cheers!
Michael N. Christoff
Site Administrator
Continuing Professional Education Online
Centre for Addiction and Mental Health
33 Russell Street
Toronto, Ontario M5S 2S1
---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org
Re: Fixes for bugs in WebdavResource putMethod and getMethod
Posted by Antoine Levy-Lambert <an...@gmx.de>.
Hello Michael,
I have just committed this fix too.
Best regards,
Antoine Levy-Lambert
On Feb 8, 2007, at 5:46 PM, Michael Christoff wrote:
> FileInputStream fis = null;
> 5: try {
> 6: setClient();
> 7: PutMethod method = new PutMethod
> (URIUtil.encodePathQuery(path));
> 8: generateIfHeader(method);
> 9: if (getGetContentType() != null && !getGetContentType
> ().equals(""))
> 10: method.setRequestHeader("Content-Type",
> getGetContentType());
> 11: long fileLength = file.length();
> 12: method.setRequestContentLength(fileLength <=
> Integer.MAX_VALUE
> 13: ? (int) fileLength
> 14: :
> PutMethod.CONTENT_LENGTH_CHUNKED);
> 15: fis = new FileInputStream(file);
> 16: method.setRequestBody(fis);
> 17: generateTransactionHeader(method);
> 18: int statusCode = client.executeMethod(method);
> 19:
> 20: setStatusCode(statusCode);
> 21: return (statusCode >= 200 && statusCode < 300) ?
> true : false;
> 22:
> 23: } finally {
> 24: if(fis != null)
> 25: fis.close();
> 26: }
Re: Fixes for bugs in WebdavResource putMethod and getMethod
Posted by Antoine Levy-Lambert <an...@gmx.de>.
Hello Michael,
I will definitively have a look at your bug reports once I get back
to work on slide. I have been distracted from my Apache work since 2
months.
Best regards,
Antoine
On Feb 8, 2007, at 5:46 PM, Michael Christoff wrote:
>
> Hi all.
>
> I just joined the list and didn't realize bug reports get sent here
> directly. So I figured I'd send my complete email directly to the
> list as well as archiving it in bugzilla.
>
> This regards some new bug fixes I coded for the Slide client API.
> The bug fixes apply to:
>
> org.apache.webdav.lib.WebdavResource.getMethod(String path, File
> file), and
> org.apache.webdav.lib.WebdavResource.putMethod(String path, File file)
>
> I had previously submitted a patch for putMethod(String, File) and
> had it accepted (see Slide 2.1 bug 40835, http://issues.apache.org/
> bugzilla/show_bug.cgi?id=40835 )--I was told it would appear in the
> next release of the API. However I see there is still a problem
> with this method even with the fix--and it turns out that the
> problem also occurs in getMethod(String, File).
>
>
> /////////// ** PUTMETHOD BUG **
>
>
> Below is the orginal code for putMethod(String, File) :
>
> ///////////////////////
>
> 1: public boolean putMethod(String path, File file)
> 2: throws HttpException, IOException {
> 3:
> 4: setClient();
> 5: PutMethod method = new PutMethod(URIUtil.encodePathQuery
> (path));
> 6: generateIfHeader(method);
> 7: if (getGetContentType() != null && !getGetContentType
> ().equals(""))
> 8: method.setRequestHeader("Content-Type",
> getGetContentType());
> 9: long fileLength = file.length();
> 10: method.setRequestContentLength(fileLength <=
> Integer.MAX_VALUE
> 12: ? (int) fileLength
> 13: :
> PutMethod.CONTENT_LENGTH_CHUNKED);
> 14: method.setRequestBody(new FileInputStream(file));
> 15: generateTransactionHeader(method);
> 16: int statusCode = client.executeMethod(method);
> 17:
> 18: setStatusCode(statusCode);
> 19: return (statusCode >= 200 && statusCode < 300) ? true :
> false;
> 20: }
>
> ///////////////////////
>
> As you can see, the FileInputStream created in line 14 is never
> closed. While this error won't manifest itself in a noticeable way
> on *nix systems, on Windows a file opened for writing is locked and
> hence applications on the client side will not be able to edit it
> from therein (one would get a "File is in use" message).
>
> The original solution was to replace line 14 with following code:
>
> 14 : FileInputStream fis = new FileInputStream(file);
> 14.1: method.setRequestBody(fis);
>
> and then insert the following code after line 18:
>
> 18.1: fis.close();
>
> However, the problem that still remains is this: what happens if an
> exception is thrown between lines 15 and 18 (inclusive)?
> Specifically, in line 16, the executeMethod of HttpClient may throw
> an IOException or HttpException. If this occurs then the
> FileInputStream will still not be closed. The solution to this is
> below:
>
>
> /////////// ** PUTMETHOD FIX **
>
>
> ///////////////////////
>
> 1: public boolean putMethod(String path, File file)
> 2: throws HttpException, IOException {
> 3:
> 4: FileInputStream fis = null;
> 5: try {
> 6: setClient();
> 7: PutMethod method = new PutMethod
> (URIUtil.encodePathQuery(path));
> 8: generateIfHeader(method);
> 9: if (getGetContentType() != null && !getGetContentType
> ().equals(""))
> 10: method.setRequestHeader("Content-Type",
> getGetContentType());
> 11: long fileLength = file.length();
> 12: method.setRequestContentLength(fileLength <=
> Integer.MAX_VALUE
> 13: ? (int) fileLength
> 14: :
> PutMethod.CONTENT_LENGTH_CHUNKED);
> 15: fis = new FileInputStream(file);
> 16: method.setRequestBody(fis);
> 17: generateTransactionHeader(method);
> 18: int statusCode = client.executeMethod(method);
> 19:
> 20: setStatusCode(statusCode);
> 21: return (statusCode >= 200 && statusCode < 300) ?
> true : false;
> 22:
> 23: } finally {
> 24: if(fis != null)
> 25: fis.close();
> 26: }
> 27: }
>
> ///////////////////////
>
> By enclosing the method's code in a try block, we can ensure fis is
> always closed, even if an exception occurs after it is instantiated.
>
>
> -------------------------
>
>
> /////////// ** GETMETHOD BUG **
>
>
> The identical problem exists in putMethod(String, File) of the same
> class. The original code is below:
>
> ///////////////////////
>
> 1: public boolean getMethod(String path, File file)
> 2: throws HttpException, IOException {
> 3:
> 4: setClient();
> 5: GetMethod method = new GetMethod(URIUtil.encodePathQuery
> (path));
> 6:
> 7: generateTransactionHeader(method);
> 8: int statusCode = client.executeMethod(method);
> 9:
> 10: setStatusCode(statusCode);
> 11:
> 12: // get the file only if status is any kind of OK
> 13: if (statusCode >= 200 && statusCode < 300) {
> 14:
> 15: // Do a simple little loop to read the response back
> into the passed
> 16: // file parameter.
> 17: InputStream inStream = method.getResponseBodyAsStream
> ();
> 18:
> 19: FileOutputStream fos = new FileOutputStream(file);
> 20: byte buffer[] = new byte[65535];
> 21: int bytesRead;
> 22: while ((bytesRead = inStream.read(buffer)) >= 0) {
> 23: fos.write(buffer, 0, bytesRead);
> 24: }
> 25: inStream.close();
> 26: fos.close();
> 27:
> 28: return true;
> 29:
> 30: } else {
> 31: return false;
> 32: }
> 33: }
>
> If an exception is thrown between lines 20 and 24 (inclusive) the
> input and output streams inStream and fos will not be closed.
>
>
>
> /////////// ** GETMETHOD FIX **
>
>
> The solution is the same as it was for putMethod(String, File) :
>
> ///////////////////////
>
> 1: public boolean getMethod(String path, File file)
> 2: throws HttpException, IOException {
> 3:
> 4: setClient();
> 5: GetMethod method = new GetMethod(URIUtil.encodePathQuery
> (path));
> 6:
> 7: generateTransactionHeader(method);
> 8: int statusCode = client.executeMethod(method);
> 9:
> 10: setStatusCode(statusCode);
> 11:
> 12: // get the file only if status is any kind of OK
> 13: if (statusCode >= 200 && statusCode < 300) {
> 14:
> 14.1: FileOutputStream fos = null;
> 14.2: InputStream inStream = null;
> 14.3: try {
> 15: // Do a simple little loop to read the response
> back into the passed
> 16: // file parameter.
> 17*: inStream = method.getResponseBodyAsStream();
> 18:
> 19*: fos = new FileOutputStream(file);
> 20: byte buffer[] = new byte[65535];
> 21: int bytesRead;
> 22: while ((bytesRead = inStream.read(buffer)) >= 0) {
> 23: fos.write(buffer, 0, bytesRead);
> 24: }
> -25-
> -26-
> 27:
> 28: return true;
> 28.1:
> 28.2: } finally {
> 28.3: if(fos != null)
> 28.4: fos.close();
> 28.5: if(inStream != null)
> 28.6: inStream.close();
> 28.7: }
> 29:
> 30: } else {
> 31: return false;
> 32: }
> 33: }
>
> ///////////////////////
>
> Lines 14.1-14.3 and 28.1-28.7 are added code, lines 25 and 26 have
> been deleted from the original source, and lines 17 and 19 have
> been updated (we now declare inStream and fos outside the try block
> in lines 14.1-14.3).
>
>
> Cheers!
>
>
>
> Michael N. Christoff
> Site Administrator
> Continuing Professional Education Online
> Centre for Addiction and Mental Health
> 33 Russell Street
> Toronto, Ontario M5S 2S1
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: slide-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org