FileItemFactory.setMaxStringLength()

• Key: FILEUPLOAD-148
• Type: New Feature
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Stepan Koltsov
• Affects Version/s: 1.2

Description

Need method

FileItemFactory.setMaxStringLength(int limitInBytes)

When this parameter is set, calling of FileItem.getString() when getSize() exceeds limitInBytes should throw Exception. This is required to avoid OOME in case of wrongly submitted forms (i. e. when bad guy puts big file into the form field "fileDescription").

Or even better sizeThreshold should be used for this value.

Comments

[Stepan Koltsov]

Alternatively method

FileItem.getString(int limitInBytes)

can be added.

[Jochen Wiedmann]

Why can't you use the fileSizeMax property?

[Stepan Koltsov]

fileSizeMax limits size of file on disk. I want limit size of FileItem.getString().

It's OK to have 2Gb file on disk, but result of getString() should be limited to 20K.

[Jochen Wiedmann]

Assuming UTF-32, which is the largest encoding I am aware of, you may set fileSizeMax to 4* stringSizeMax and have an effective limit. Ok, that's not a hard limit, because the user might use a more efficient encoding in terms of space, but that should be fine, IMO.

[Stepan Koltsov]

No, I want to limit data allowed to be loaded into memory.

I want limit files to 200M and limit size of data in memory to 20K, and I want to avoid explicit checking of getSize() each time before calling getString().

Even better, FileItem.get() should throw exception if file is exceeds some parameter.

Point is to deny loading large files into memory from FileItem.get() method.

Currently, calling of 200M FileItem.get() (or getString()) causes OutOfMemoryError.

[Jochen Wiedmann]

> Point is to deny loading large files into memory from FileItem.get() method.

> Point is to deny loading large files into memory from FileItem.get() method.

That's what DiskFileItemFactory.setThreshold() gives you. It seems, you are not using this property or are setting it to a non-sensible value.

[Stepan Koltsov]

setThreshold() causes large files to be written on disk. But after files is saved to disk, calling of FileItem.get() loads that large item into memory:

===
public byte[] get() {
if (isInMemory()) {
if (cachedContent == null) { cachedContent = dfos.getData(); }
return cachedContent;
}

byte[] fileData = new byte[(int) getSize()];
FileInputStream fis = null;

try { fis = new FileInputStream(dfos.getFile()); fis.read(fileData); } catch (IOException e) { fileData = null; } finally {
if (fis != null) {
try { fis.close(); } catch (IOException e) { // ignore }
}
}

return fileData;
}
===

[Jochen Wiedmann]

How about using FileItem.getSize()? Or using a custom FileItemFactory that meets your wished?

[Stepan Koltsov]

I think this functionality would be useful to all users. Because almost nobody checks file.getSize() before calling getString(), so almost any site that uses commons-fileupload can be DOSed by uploading big flie into "text" field.