You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Amichai Rothman (JIRA)" <ji...@apache.org> on 2006/05/19 12:22:29 UTC
[jira] Created: (FILEUPLOAD-110) MultipartStream's keep region
padding is either unnecessary or untested (and undocumented)
MultipartStream's keep region padding is either unnecessary or untested (and undocumented)
------------------------------------------------------------------------------------------
Key: FILEUPLOAD-110
URL: http://issues.apache.org/jira/browse/FILEUPLOAD-110
Project: Commons FileUpload
Type: Bug
Versions: 1.1 Final
Reporter: Amichai Rothman
Priority: Minor
MultipartStream has logic and constants related to a "keep region" which, according to the docs, is
"The amount of data, in bytes, that must be kept in the buffer in order to detect delimiters reliably."
However, why that region is needed, why the padding is set to 3, and what makes it more reliable, is undocumented. Furthermore, when setting KEEP_REGION_PAD to zero (which effectively bypasses the extra keep region padding mechanism - it simply uses the boundary delimiter size, which makes sense), all tests pass successfully.
so... either the extra padding is required but whatever it is required for is untested and undocumented, which should be corrected, or it is indeed unneeded, in which case all the keep region related code and constants can be deleted, and the code where the actual delimiters are searched for can be modified to simply use the boundary length instead of keepRegion.
Note: I suspect the keep region pad may be a patch to compensate for the skipPreamble() patch which modifies the global boundary, calls a method which uses it, and the restores the global variable. If this is the case, this can all be reorganized in a clear and straightforward manner (with no awkward patches) by using an internal utility method that simply reads data into a given outputstream until a given delimiter is reached. this can then be used by readHeaders, readBodyData, discardBodyData, and skipPreamble, all of which require this same basic underlying functionality. I'd be happy to provide this code change, if this seems like a correct analysis to you...
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
[jira] Resolved: (FILEUPLOAD-110) MultipartStream's keep region
padding is either unnecessary or untested (and undocumented)
Posted by "Jochen Wiedmann (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FILEUPLOAD-110?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jochen Wiedmann resolved FILEUPLOAD-110.
----------------------------------------
Resolution: Fixed
Assignee: Jochen Wiedmann
The extra bytes are no longer used.
> MultipartStream's keep region padding is either unnecessary or untested (and undocumented)
> ------------------------------------------------------------------------------------------
>
> Key: FILEUPLOAD-110
> URL: https://issues.apache.org/jira/browse/FILEUPLOAD-110
> Project: Commons FileUpload
> Issue Type: Bug
> Affects Versions: 1.1 Final
> Reporter: Amichai Rothman
> Assignee: Jochen Wiedmann
> Priority: Minor
> Attachments: commons-fileupload-1.1-bug-removed-keep-region.patch
>
>
> MultipartStream has logic and constants related to a "keep region" which, according to the docs, is
> "The amount of data, in bytes, that must be kept in the buffer in order to detect delimiters reliably."
> However, why that region is needed, why the padding is set to 3, and what makes it more reliable, is undocumented. Furthermore, when setting KEEP_REGION_PAD to zero (which effectively bypasses the extra keep region padding mechanism - it simply uses the boundary delimiter size, which makes sense), all tests pass successfully.
> so... either the extra padding is required but whatever it is required for is untested and undocumented, which should be corrected, or it is indeed unneeded, in which case all the keep region related code and constants can be deleted, and the code where the actual delimiters are searched for can be modified to simply use the boundary length instead of keepRegion.
> Note: I suspect the keep region pad may be a patch to compensate for the skipPreamble() patch which modifies the global boundary, calls a method which uses it, and the restores the global variable. If this is the case, this can all be reorganized in a clear and straightforward manner (with no awkward patches) by using an internal utility method that simply reads data into a given outputstream until a given delimiter is reached. this can then be used by readHeaders, readBodyData, discardBodyData, and skipPreamble, all of which require this same basic underlying functionality. I'd be happy to provide this code change, if this seems like a correct analysis to you...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org