Uploaded image for project: 'Commons FileUpload'
  1. Commons FileUpload

MultipartStream's keep region padding is either unnecessary or untested (and undocumented)



    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 1.1 Final
    • None
    • None


      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...




            jochen@apache.org Jochen Wiedmann
            amichai Amichai Rothman
            0 Vote for this issue
            1 Start watching this issue