[ 
https://issues.apache.org/jira/browse/FILEUPLOAD-130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12484525
 ] 

Michael Macaluso commented on FILEUPLOAD-130:
---------------------------------------------

A couple of comments:

1) I don't think this patch file was complete because it affected things 
outside of CVS.  Just to clarify, I think you also renamed 
MutableFileItemHeadersImpl.java to FileItemHeadersImpl.java, removed 
MutableFileItemHeaders.java, removed Headers.java and removed 
FileItemHeadersTest.java.

2) This patch removes an ability for a subclass to override the FileItemHeaders 
implementation.  I am assuming that is why the MutableFileItemHeaders concepts 
was dropped.  This is a minor point, but in general, we find more flexibility 
when things can be overridden without copying the logic.  Possibly adding a 
protected "factory" method for returning a new FileItemHeadersImpl in 
FileUploadBase.java might accomplish this task. Additionally, we may want to 
move the FileItemHeadersImpl class to a protected static class within 
FileUploadBase.

3) Since FileItemStream is only, and can only be, implemented within this 
package, we should sub-type FileItemStream from FileItemHeaderSupport, or add 
the getHeaders() method to the interface itself.  This will mean that packages 
that use the streaming interface will automatically get the ability to process 
the headers.

4) The utils\Headers.java class greatly simplified code (both for end-users and 
internal code) by removing/centralizing the instanceof and casting operations.  
You could argue against this (especially if #3 is implemented), but it might 
support adoption.

Thanks,
Michael

> Add ability to get any header from the FileItem and FileItemStream interfaces
> -----------------------------------------------------------------------------
>
>                 Key: FILEUPLOAD-130
>                 URL: https://issues.apache.org/jira/browse/FILEUPLOAD-130
>             Project: Commons FileUpload
>          Issue Type: Improvement
>    Affects Versions: 1.2
>            Reporter: Michael Macaluso
>            Priority: Minor
>         Attachments: FILEUPLOAD-130.patch, FileUpload-130_1.patch, 
> FileUpload-130_2.patch
>
>
> The FileItem and FileItemStream interfaces should have a way to return back 
> any header that was encountered during the header parsing for an "Item".  
> Currently, from the FileItemStatus you can only get information from the 2 
> pre-defined headers "Content-Type" and "Content-Disposition" (Sort-of because 
> the header can not be accessed raw).  Other than the interface changes 
> (including the change to pass them along in the FileItemFactory interface), 
> it appears that all changes can be made within the FileUploadBase.java file.  
> FileUploadBase.java:859 (as of 1.2) has the headers, but the call to create 
> the FileItemStreamImpl on lines 877 and 887 do not include the headers map.  
> Further, the parseRequest method uses the FileItemStream interface to build 
> the FileItem, so you should always have the headers in question.
> The reason for this request is that we have an application that is sending 
> per-part headers (not precluded by the specs as far as we know of) to provide 
> more information than name and content-type and using the FileUpload project 
> means that we can no longer find out those header values.
> [Also, not completely sure, but I believe FileUploadBase.createItem(Map, 
> boolean) on line 480 is not referenced anymore in this project.]

-- 
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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to