Executive Summary:
Ed suggests we rewite the javadoc to explicitly allow or disallow the
corner cases mentioned in the thread.
Details:
>>>>> On Wed, 22 Mar 2017 09:25:52 -0700, Edward Burns <edward.burns_at_oracle.com> said:
EB> I had the pleasure of discussing this in person with Simone Bordet at
EB> Devoxx US yesterday. Unfortunately, we didn't come to any conclusion
EB> about the best way forward. Shing-wai and I are still discussing it.
>>>>> On Wed, 8 Mar 2017 16:09:16 +0900, Greg Wilkins <gregw_at_webtide.com> said:
GW> Janb has asked me to seek clarification on the Part.write(String) method.
GW> The spec for @MultipartConfiguration.location says:
Spec8.1.5> The location attribute of the
Spec8.1.5> javax.servlet.annotation.MultipartConfig and the <location>
Spec8.1.5> element of the <multipart-config> is interpreted as an
Spec8.1.5> absolute path and defaults to the value of the
Spec8.1.5> javax.servlet.context.tempdir. If a relative path is
Spec8.1.5> specified, it will be relative to the tempdir location. The
Spec8.1.5> test for absolute path vs relative path MUST be done via
Spec8.1.5> java.io.File.isAbsolute.
GW> The javadoc for Part.write(String fileName) says:
Part.write> fileName - the name of the file to which the stream will be
Part.write> written. The file is created relative to the location as
Part.write> specified in the MultipartConfig
Greg then shares Tomcat's approach:
GW> Tomcat have changed the javadoc on their version of the API to say:
Tomcat> @param fileName The location into which the uploaded part should
Tomcat> be stored. Relative locations are relative to {_at_link
Tomcat> javax.servlet.MultipartConfigElement#getLocation()}
GW> I don't mind the Tomcat version
>>>>> On Thu, 9 Mar 2017 07:20:53 +1100, Stuart Douglas <sdouglas_at_redhat.com> said:
SD> For what it is worth we also implement the Tomcat behavior.
This is an improvement on the version in the Servlet spec, but it still
leaves some things unspecified:
GW> It is not clear whether fileName should be interpreted as a Path or
GW> not
GW> what should be the result if it specifies an absolute location
GW> what should be the result if it contains special path elements such
GW> as "." or "..".
GW> security concerns about allowing the container to try to write to
GW> any old path)
GW> Should an absolute filename throw an IAE or should it just be interpreted
GW> relative to the config location? What if the filename is something like
GW> "C:\\foo\bar"?
I am hesitant to tighten the set of allowable values to Part.write()
such that things that used to work cease working, with the exception of
using "../" to escape one web app and write into the space of another.
In this case, I think a better approach is to rewite the javadoc to
explicitly allow or disallow the corner cases mentioned above.
PROPOSAL: Modify the spec for Part.write() to be:
Add this text in the main javadoc for the method.
-----
The following constraints apply to the fileName argument:
* For all values of the fileName argument, the resultant location on the
server's filesystem is relative to the location as specified in the
MultipartConfig.
* If fileName is an absolute location, it still is relative to the
location, in a system-dependent manner, as specified in the
MultiPartConfig.
* If fileName contains system-dependent relative path segments, such as
"../", if the resultant path is "higher" than the location, as
specified in the MultiPartConfig, throw an IOException.
----
Change the paramaters to be:
Parameters:
fileName - the path of the file to which the stream will be written.
Throws:
IOException - if an error occurs.
What do you think?
ACTION: Please respond by close of business Monday 27 March. In the
absence of feedback, we'll just go with this proposal.
Thanks,
Ed
--
| edward.burns_at_oracle.com | office: +1 407 458 0017
| 3 business days until JavaLand 2017
| 21 business days until planned start of Servlet 4.0 Public Review