Binary File Upload using Restlet + Apache Commons FileUpload

[santiagopoli]

I have a REST API with Restlet 2.3 and need to implement a file-uploading functionality to it.
The problem is that when someone uploads a file using a POST (with a multipart/form-data Content-Type), the file reaches the server with another encoding. To test this, I printed the contents of the original file in a Unix Terminal and then printed it again before parsing the requet with Apache Commons FileUpload (with almost the same code of this example http://restlet.com/technical-resources/restlet-framework/guide/2.2/extensions/fileupload). Both printed contents are very similar, but the original file has less characters, so i assume that my Java server is using the wrong encoding to interpret the file.

The file I sent is a PNG image. With text files the server works perfectly, but when I send photos or any binary file, the problem appears.

[santiagopoli]

Here is the method which implements this functionality.

private String getFileData(FileItemIterator fileIterator) throws IOException {
    String result = null;
    while (fileIterator.hasNext()) {
        FileItemStream fi = fileIterator.next();
        if (fi.getFieldName().equals(FILE_TO_UPLOAD)) {
            StringBuilder sb = new StringBuilder();
            BufferedReader br = new BufferedReader(new InputStreamReader(fi.openStream()));
            String line = null;
            while ((line = br.readLine()) != null) {
                sb.append(line);
            }
            sb.append("\n");
            result = sb.toString();
        }
    }
    return result;
}

[santiagopoli]

Actually solved it by using Google's ByteStreams class:

while (fileIterator.hasNext()) {
    FileItemStream fi = fileIterator.next();
    if (fi.getFieldName().equals(FILE_TO_UPLOAD)) {
        byte[] byteArray = ByteStreams.toByteArray(fi.openStream());
        result = new String(byteArray,Charset.forName("ISO-8859-1"));
    }
}

[Tembrel]

That's a little dangerous, since you aren't closing the InputStream.

How about:

    while (fileIterator.hasNext()) {
        FileItemStream fi = fileIterator.next();
        if (fi.getFieldName().equals(FILE_TO_UPLOAD)) {
            Optional<String> str = readAsString(fi);
            if (str.isPresent()) {
                result = str.get();
            } else {
                // ???
            }
        }
    }

where readAsString is defined using try-with-resources:

private Optional<String> readAsString(FileItemStream fi) {
    try (InputStream s = fi.openStream()) {
        return Optional.of(new String(ByteStreams.toByteArray(s), Charsets.ISO_8859_1));
    } catch (IOException ex) {
        System.err.printf("problem opening or reading %s: %s", fi.getName(), ex);
        return Optional.absent();
    }
}

--tim

On Thu, Feb 5, 2015 at 9:09 AM, Santiago Ignacio Poli <
notifications@github.com> wrote:

Actually solved it using Google's ByteStreams class:

while (fileIterator.hasNext()) {
FileItemStream fi = fileIterator.next();
if (fi.getFieldName().equals(FILE_TO_UPLOAD)) {
byte[] byteArray = ByteStreams.toByteArray(fi.openStream());
result = new String(byteArray,Charset.forName("ISO-8859-1"));
}
}

—
Reply to this email directly or view it on GitHub
#1023 (comment)
.

[santiagopoli]

Thanks for your answer. Yes, I did notice that I didn't close the stream, so I changed my code to this:

while (fileIterator.hasNext()) {
    FileItemStream fi = fileIterator.next();
    if (fi.getFieldName().equals(FILE_TO_UPLOAD)) {
        InputStream attachmentStream = fi.openStream();
        byte[] attachmentBytes = ByteStreams.toByteArray(attachmentStream);
        attachmentStream.close();
        result = new String(attachmentBytes, Charset.forName("ISO-8859-1"));
    }
}

I didn't try your solution though, but it seems solid.

[Tembrel]

That's going to fail to close the stream if there is an exception thrown
during toByteArray. If this happens enough you could start to leak
connections.

Why not use try-with-resources? That's what it's for!

--tim

On Thu, Feb 5, 2015 at 9:48 AM, Santiago Ignacio Poli <
notifications@github.com> wrote:

Thanks for your answer. Yes, I did notice that i didn't close the stream,
so I changed my code to this:

while (fileIterator.hasNext()) {
FileItemStream fi = fileIterator.next();
if (fi.getFieldName().equals(FILE_TO_UPLOAD)) {
InputStream attachmentStream = fi.openStream();
byte[] attachmentBytes = ByteStreams.toByteArray(attachmentStream);
attachmentStream.close();
result = new String(attachmentBytes, Charset.forName("ISO-8859-1"));
}
}

—
Reply to this email directly or view it on GitHub
#1023 (comment)
.

[santiagopoli]

I did modify my code to make it more understandable. The real method stands like this:

private AttachmentData getFileData(FileItemIterator fileIterator) throws FileUploadException, IOException {
    AttachmentData result = null;
    while (fileIterator.hasNext()) {
        FileItemStream fi = fileIterator.next();
        if (fi.getFieldName().equals(FILE_TO_UPLOAD)) {
            InputStream attachmentStream = fi.openStream();
            try {
                byte[] attachmentBytes = ByteStreams.toByteArray(attachmentStream);
                result = new AttachmentData(attachmentBytes);
            } finally {
                attachmentStream.close();
            }
        }
    }
    return result;
}

[santiagopoli]

I didn't knew about try-with-resources (http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html). All of our projects use Java 7, so I've always used a try-catch-finally or try-finally statement.
It seems that I learn something new everyday :)

[thboileau]

@santiagopoli , do you think we can close this issue?

I wonder if your initial issue is that you were reading binary content in decoding it into characters.
I'm not sure to understand your last comment, butI think you can use try-with-resource with jdk7.