RE: [Springframework-developer] Multipart committed

[Alef A. \(JTeam\) <al...@jt...>]

Sounds good,

In one of our projects I have to add fileupload functionality soon, so
as soon as I have a chance, I'll have a look at it!

Alef

-----Oorspronkelijk bericht-----
Van: spr...@li...
[mailto:spr...@li...] Namens
Trevor Cook
Verzonden: Tuesday, September 30, 2003 3:05 AM
Aan: j=FCrgen h=F6ller [werk3AT]; al...@jt...; Spring Developers
Onderwerp: [Springframework-developer] Multipart committed


I've just committed the multipart stuff to cvs.  It currently should NOT
change any existing projects.  In order to "activate" it you will need
to explicitly turn it on as outlined below. <WARNING>Note that these
procedures for enabling it are subject to change, as is the underlying
implementation.  Please consider the entire multipart implementation
highly volatile until further notice.</WARNING>

With that out of the way, I'll detail how everything works.  I've
seperated it into 3 parts, the "multipart api", changes to Spring, and
the jakarta commons fileupload implementation.  Note that I've provided
what I consider a sufficient level of documentation in the javadocs, so
if anyone referring to those finds them lacking, please let me know
(since I think improving our docs is/should be a big push right now).
I'd also appreciate any help/suggestions on how to unit test these,
since I'm not exactly sure how to proceed with them (due to how to make
mock request, etc.).


Multipart API
-------------
The api contains 3 classes and 1 exception:
- org.springframework.web.servlet.MultipartResolver
- org.springframework.web.servlet.MultipartHttpServletRequest
- org.springframework.web.servlet.MultipartFile
- org.springframework.web.servlet.MultipartException

Most of the details for these are in the javadocs.  The interfaces are
in the org.springframework.web.servlet package to follow the pattern set
by the LocaleResolver/ThemeResolver.

I made the content for the MultipartFile a byte array (byte[]) and
avoided using File/InputStream.  This seems to make it simpler and more
consistent. If you need to retrieve the original filename, you will have
to use a MultipartFile object in your bean.  Based on my personal
feelings and comments from others, I think using a File is inappropriate
(except "under the covers" when necessary), and the only other
alternative is to create some form of "NamedByteArray".  Since any
special class containing a filename will introduce a Spring dependancy,
I think MultipartFile is as good as creating an additional class.

The methods in the MultipartHttpServletRequest were designed to mimic
their HttpServletRequest parameter counterparts.  The only one not
matched currently is "getFileValues()" to cover the possiblity of a
MultipartFile array.  What are the opinions on supporting a
MultipartFile array.  I personally have never used them because they add
a ton of extra complexity. With Spring, it might not be so bad now, but
I'd have to see a need.  If we do see a potential for using them later,
we could create the interface declaration now, and use a nop exception
in the implementation until we get it working.  Personally,  I'd leave
it out, I'm mentioning it in case others see/have real uses for it.


Changes to Spring
-----------------
Just to reiterate, none of these changes should mess up any existing
Spring-based apps.  Also, all the Spring code is based on the above api.
None of it references any of the "commons" implementation, so it should
be fully pluggable for other implementations.

org.springframework.web.servlet.DispatcherServlet looks for a bean named
"multipartResolver" and if it doesn't find one, it does NOT create a
default (it uses null).  The reason for this is because we don't want to
wrap normal request's so that we can use "request instanceof
MultipartHttpServletRequest".  If it contains a multipartResolver, it
wraps the request ONLY if "multipartResolver.isMultipart(request)" is
true.  This wrapped request is then used for the balance of the
doService method, and then cleanup is performed.  This means that if no
MultipartResolver is set, no multipart wrapping/processing will occur.

org.springframework.web.bind.ServletRequestDataBinder has a new
constructor which takes the request as a parameter.  This is to allow
special multipart property editors to be assigned.  I tried various
methods of having the binder simply use the normal getParameter method
to parse the content, but it proved very clumsy and didn't meet all
needs (like getting filename).  In the end it appears that the property
editor needs access to the request so it can map "getParamater(String)"
results to "getFile(String)" results. This behaviour is documented in
the MultipartResolver.resolveMultipart(HttpServletRequest)" javadocs.
This means that wherever you currently create a ServletRequestDataBinder
you need to use the new constructor if you want it to be able to use the
multipart handling.

Unless somebody can see major flaws in using this approach or has a more
elegant solution, I would recommend deleting the current constructor and
have all methods use the new constructor which receives the request.
The way the constructor is written, if a multipartresolver isn't set,
things occur the same way they do now.  Basically, using the new
constructor won't change anything unless multipart handling is used (in
which case we need this constructor anyway).

"getMultipartResolver(HttpServletRequest)" was added to
org.springframework.web.servlet.support.RequestContextUtils.  This is
just a utility method and is only used (so far) by
ServletRequestDataBinder.

I created a few "general use" multipart classes (all of which are in the
org.springframework.web.servlet.multipart package).  The main one is
MultipartHttpServletRequestImpl.  It should work for most multipart
implementations unless someone has very special requirements (since most
"real work" occurs in the resolver).  I also created 3 property editors
(which are all used by the commons implementation, but can be used by
others as well). MultipartBytePropertyEditor supports "byte[]" and maps
the content
(MultipartFile.getBytes()) as the value.  MultipartFilePropertyEditor
supports "MultipartFile" and maps the whole MultipartFile object as the
value.  MultipartStringPropertyEditor supports String and maps the
filename
(MultipartFile.getFileName()) as the value.  All 3 are reusable for any
other multipart implementations, and it should be simple to create
custom property editors if desired (like File, etc.).

The String editor is the one I question a little, so I'd appreciate
feedback.  Basically, if you don't have a String editor, then the
"key=3Dvalue" requirement means you will get weird values in your =
objects.
Placing the file content in a String doesn't seem to make sense either.
So basically, this seemed like the "better of evils" (a great design
justification :) ).


Jakarta Commons FileUpload implementation
-----------------------------------------
The implementation only requires 2 special classes, CommonsMultipartFile
and CommonsMultipartResolver (both in
org.springframework.web.servlet.multipart
package).  CommonsMultipartFile is very basic, just a simple wrapper
around org.apache.commons.fileupload.FileItem .

CommonsMultipartResolver uses commons to process isMultipart requests,
and it registers the 3 editors mentioned above.  The actual
resolveMultipart contains special handling to check each FileItem in
order to ensure "regular field" arrays are handled correctly.  It
provides 4 configuration settings which can be set in the context file,
etc. which are specific to how the commons package works.  The default
values are outlined in the javadocs, but I'll list them here for
discussion:

maximumFileSize =3D -1 (no file size limit)
maximumInMemorySize =3D 1024 (1 kb)
temporaryFilePath =3D System.getProperty("java.io.tmpdir")

These are all consistent with the actual commons defaults.  The only
questionable one is the "maximumFileSize" being unlimited (since this
could be a DOS/DDOS hole as Colin mentioned earlier).  However, I don't
think we can choose an arbitrary default that will work for a "majority"
of uses.  I would recommend that we simply leave it as is and let the
user set it, but I'm open to suggestions.


Conclusion/Questions
--------------------
I have already converted our big app and everything works fine with
these classes, including validation (we actually check the byte[] to
make sure it is an image in one case).  Once we finalize the api and
default implementation, what should Spring's default be, either no
multipart handling or the commons implementation?  Personally, I think
having it on by default makes sense, since you'll never use it unless
you receive a form which is multipart (in which case you need it
anyway).  What does everyone else think?


Trevor D. Cook



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf _______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer