Re: [IO] Change in behavior in Commons FileUpload after upgrade to Commons IO 2.16.1
Stephan, Thank you for your message. This is more of a design deffect IMO,. If there is a desire for disabling a feature like caching then there should be a toggle for that, not relying on a side effect of a magic number. PRs welcome! :-) Gary On Wed, Apr 10, 2024, 9:24 AM Stephan Markwalder wrote: > Hi, > > Today, I found the following questions in > https://github.com/apache/commons-io/pull/609: > > > The behavior for a negative threshold should be the same as 0 IMO. WDYT? > > What does it even mean that a threshold is negative? > > Writing zero bytes writes nothing, so there is nothing to reach until > you at least write one byte. > > Am I missing something? > > I would like to highlight a "use case" for a negative threshold, and how > the change to disallow a negative threshold might impact existing code. > > I upgraded from Commons IO 2.15.1 to 2.16.1 yesterday and found a small > change in the behavior of Commons FileUpload when uploading and processing > empty files. The effect is visible only when passing a negative value as > file size threshold to Commons FileUpload. Here is a small extract from the > Javadoc of Commons FileUpload, class `DefaultFileItemFactory`, constructor > parameter `sizeThreshold`: > > > sizeThreshold - The threshold, in bytes, below which items will be > > retained in memory and above which they will be stored as a file. > (source: > https://javadoc.io/doc/commons-fileupload/commons-fileupload/latest/index.html > ) > > By passing a negative value for `sizeThreshold`, Commons FileUpload can be > configured to disable the in-memory caching for all uploaded files, > including empty files with a size of 0 bytes. As a result, `DiskFileItem` > objects created by Commons FileUpload will always have a `File` instance > set internally, even for empty files. > > `DiskFileItem` in Commons FileUpload internally makes use of > `DeferredFileOutputStream`, and therefore `ThresholdingOutputStream`. At > some point it calls `isThresholdExceeded()` to check whether the size of > the uploaded file exceeds the given threshold. By disallowing a negative > threshold, empty files will now be treated differently by Commons > FileUpload. With a size of 0 bytes, they will not exceed the enforced > minimum threshold of 0 bytes anymore, and their data will therefore be kept > in memory. This can break follow-up code which relies on the previous > behavior and expects a `File` instance to be created for every uploaded > file, even empty files. > > I know that this is a very specific use case. I don't know whether the > developers of Commons FileUpload ever intended a negative threshold to be > used. Still, the question was asked whether a negative threshold could have > any meaning. I assume the answer is "yes". But I don't know whether this > qualifies as a bug or a regression. I also don't know whether there are > other similar use cases in other libraries depending on Commons IO. > > Best, > Stephan > Email Disclaimer > FNZ (UK) Ltd registered in England and Wales (05435760) 10th Floor, 135 > Bishopsgate, London EC2M 3TP, FNZ (UK) Ltd is authorised and regulated by > the Financial Conduct Authority (438687); FNZ TA Services Ltd registered in > England and Wales (09571767) 10th Floor, 135 Bishopsgate, London EC2M 3TP, > FNZ TA Services Ltd is authorised and regulated by the Financial Conduct > Authority (932253); FNZ Securities Ltd registered in England and Wales > (09486463) 10th Floor, 135 Bishopsgate, London EC2M 3TP, FNZ Securities > Ltd, is authorised and regulated by the Financial Conduct Authority > (733400); JHC Systems Limited registered in England and Wales (08729370) > Temple Point 6th Floor, 1 Temple Row, Birmingham, West Midlands, B2 5LG; > FNZ (Europe) DAC registered in Ireland (657886) Block C, Irish Life > Centre, Lower Abbey Street, Dublin 1, D01V9F5, Ireland; FNZ SA (Pty) Ltd > registered under the laws of South Africa (2018/547997/07), 1st floor, > Newport House, Prestwich Street, Greenpoint, western Cape, 8001; FNZ > Limited registered in New Zealand (1797706) FNZ House, Level 3, 29A Brandon > Street, Wellington, 6011 New Zealand; FNZ (Australia) Pty Ltd registered in > Australia (138 819 119) Level 1, 99 Elizabeth St, Sydney 2000; FNZ (Hong > Kong) Limited registered in Hong Kong (1305362) 6A-1, Koshun House, 331 > Nathan Road, Hong Kong; FNZ (Singapore) Services Pte. Ltd. registered in > Singapore (201307199R) 61 Robinson Road, #13-03A, Robinson Centre, > Singapore (068893); and FNZ (China) Ltd registered in China > (91310115MA1K3G4K6T) [中国（上海）自由贸易试验区世纪大道1196 号二 座20 层. > This message is intended solely for the addressee and may contain > confidential information. If you have received this message in error, > please send it back to us, and immediately and permanently delete it. Do > not use, copy or disclose the information contained in this message or in > any attachment. > Emails sent to and from FNZ may be monitored and read for legitimate > business purposes. Emails
[IO] Change in behavior in Commons FileUpload after upgrade to Commons IO 2.16.1
Hi, Today, I found the following questions in https://github.com/apache/commons-io/pull/609: > The behavior for a negative threshold should be the same as 0 IMO. WDYT? > What does it even mean that a threshold is negative? > Writing zero bytes writes nothing, so there is nothing to reach until you at > least write one byte. > Am I missing something? I would like to highlight a "use case" for a negative threshold, and how the change to disallow a negative threshold might impact existing code. I upgraded from Commons IO 2.15.1 to 2.16.1 yesterday and found a small change in the behavior of Commons FileUpload when uploading and processing empty files. The effect is visible only when passing a negative value as file size threshold to Commons FileUpload. Here is a small extract from the Javadoc of Commons FileUpload, class `DefaultFileItemFactory`, constructor parameter `sizeThreshold`: > sizeThreshold - The threshold, in bytes, below which items will be > retained in memory and above which they will be stored as a file. (source: https://javadoc.io/doc/commons-fileupload/commons-fileupload/latest/index.html) By passing a negative value for `sizeThreshold`, Commons FileUpload can be configured to disable the in-memory caching for all uploaded files, including empty files with a size of 0 bytes. As a result, `DiskFileItem` objects created by Commons FileUpload will always have a `File` instance set internally, even for empty files. `DiskFileItem` in Commons FileUpload internally makes use of `DeferredFileOutputStream`, and therefore `ThresholdingOutputStream`. At some point it calls `isThresholdExceeded()` to check whether the size of the uploaded file exceeds the given threshold. By disallowing a negative threshold, empty files will now be treated differently by Commons FileUpload. With a size of 0 bytes, they will not exceed the enforced minimum threshold of 0 bytes anymore, and their data will therefore be kept in memory. This can break follow-up code which relies on the previous behavior and expects a `File` instance to be created for every uploaded file, even empty files. I know that this is a very specific use case. I don't know whether the developers of Commons FileUpload ever intended a negative threshold to be used. Still, the question was asked whether a negative threshold could have any meaning. I assume the answer is "yes". But I don't know whether this qualifies as a bug or a regression. I also don't know whether there are other similar use cases in other libraries depending on Commons IO. Best, Stephan Email Disclaimer FNZ (UK) Ltd registered in England and Wales (05435760) 10th Floor, 135 Bishopsgate, London EC2M 3TP, FNZ (UK) Ltd is authorised and regulated by the Financial Conduct Authority (438687); FNZ TA Services Ltd registered in England and Wales (09571767) 10th Floor, 135 Bishopsgate, London EC2M 3TP, FNZ TA Services Ltd is authorised and regulated by the Financial Conduct Authority (932253); FNZ Securities Ltd registered in England and Wales (09486463) 10th Floor, 135 Bishopsgate, London EC2M 3TP, FNZ Securities Ltd, is authorised and regulated by the Financial Conduct Authority (733400); JHC Systems Limited registered in England and Wales (08729370) Temple Point 6th Floor, 1 Temple Row, Birmingham, West Midlands, B2 5LG; FNZ (Europe) DAC registered in Ireland (657886) Block C, Irish Life Centre, Lower Abbey Street, Dublin 1, D01V9F5, Ireland; FNZ SA (Pty) Ltd registered under the laws of South Africa (2018/547997/07), 1st floor, Newport House, Prestwich Street, Greenpoint, western Cape, 8001; FNZ Limited registered in New Zealand (1797706) FNZ House, Level 3, 29A Brandon Street, Wellington, 6011 New Zealand; FNZ (Australia) Pty Ltd registered in Australia (138 819 119) Level 1, 99 Elizabeth St, Sydney 2000; FNZ (Hong Kong) Limited registered in Hong Kong (1305362) 6A-1, Koshun House, 331 Nathan Road, Hong Kong; FNZ (Singapore) Services Pte. Ltd. registered in Singapore (201307199R) 61 Robinson Road, #13-03A, Robinson Centre, Singapore (068893); and FNZ (China) Ltd registered in China (91310115MA1K3G4K6T) [中国（上海）自由贸易试验区世纪大道1196 号二 座20 层. This message is intended solely for the addressee and may contain confidential information. If you have received this message in error, please send it back to us, and immediately and permanently delete it. Do not use, copy or disclose the information contained in this message or in any attachment. Emails sent to and from FNZ may be monitored and read for legitimate business purposes. Emails cannot be guaranteed to be secure or error-free, and you should protect your systems. FNZ does not accept any liability arising from interception, corruption, loss or destruction of this email, or if it arrives late or incomplete or with errors or viruses. For more information about the FNZ group please visit our website here where you can also find links to our policies including our Privacy policy which
