[MAGNOLIA-540] Replacing default multipart filter implementation (problematic com.oreilly.servlet package license terms) Created: 06/Sep/05  Updated: 26/Jun/19  Resolved: 25/Nov/13

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 2.1 Final
Fix Version/s: None

Type: Task Priority: Major
Reporter: Andreas Brenk Assignee: Fabrizio Giustina
Resolution: Outdated Votes: 0
Labels: None
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: Not Specified Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Attachments: Java Source File MultipartRequestFilter.java     Java Source File MultipartRequestFilterTest.java    
Issue Links:
Relates
relation
is related to MAGNOLIA-1811 MultipartRequestFilter (impl. based o... Closed
is related to MAGNOLIA-2103 Can't Create New Paragraph With Safar... Closed
Sub-Tasks:
Key
Summary
Type
Status
Assignee
MAGNOLIA-2033 MultipartRequestFilter chockes on req... Sub-task Closed Philipp Bärfuss  
Template:
Acceptance criteria:
Empty
Task DoR:
Empty
Date of First Response:

 Description   

The WAR file download contains Jason Hunter's com.oreilly.servlet package in the WEB-INF/lib directory. Its license terms (see http://servlets.com/cos/license.html) are problematic I fear.

Especially items 5 and 6 are currently violated:

"5. The distribution includes copyright notice as follows: "The source code, object code, and documentation in the com.oreilly.servlet package is copyright and owned by Jason Hunter." in the documentation and/or other materials provided with the distribution.

6. You reproduce the above copyright notice, this list of conditions, and the following disclaimer in the documentation and/or other materials provided with the distribution."

Both "license.txt" and "release notes.txt" contained in the WAR file download do not mention anything demanded by the terms cited above.

A problematic second point is the license for commercial usage and redistribution as every developer must own a copy of Jason's book or negotiate a seperate license.

Jakarta Commons FileUpload (http://jakarta.apache.org/commons/fileupload/) is a possible replacement at least for some functionality.



 Comments   
Comment by Boris Kraft [ 08/Sep/05 ]

Andreas, thanks for pointing this out - for us it was not problematic because we own a copy of the book

Do you have time to check if the alternatives would suffice, and provide a patch?
Thanks, Boris

Comment by Andreas Brenk [ 09/Sep/05 ]

Okay, I replaced the MultipartRequestFilter with a new implementation using commons-fileupload.

You have to remove the dependency on cos and a one on commons-fileupload to project.xml:

<dependency>
<groupId>commons-fileupload</groupId>
<artifactId>commons-fileupload</artifactId>
<version>1.0</version>
<type>jar</type>
<url>http://jakarta.apache.org/commons/fileupload/</url>
<properties>
<war.bundle>true</war.bundle>
</properties>
</dependency>

Warning: I haven't tested the new code, yet. I'm currently using the WAR distribution and have to complete my Magnolia build environment first...

Comment by Fabrizio Giustina [ 12/Sep/05 ]

Just a comment: I already tried replacing cos with commons-fileupload months ago, during the dependencies cleanup work, but my implementation failed in 2 points:

  • UTF8 support was not working properly (well, the UTF8 transition was not completed yet, so probably there were other problems kicking in)
  • handling of multi-values parameter (I can't really remember well the issue I had, but just be sure to test this properly)

said that, I'm all for migrating to commons-fileupload and I will try to help with the implementation: we could start adding the dependency and committing a new multipart filter along with the existing one for tests (leaving the old one configured by default while testing the new one)... I'll do that

Comment by Andreas Brenk [ 12/Sep/05 ]

A first go at a JUnit test case.

Comment by Andreas Brenk [ 12/Sep/05 ]

I forgot: the test case uses commons-httpclient 3.0-rc3. So you have to update the dependency.

Multi-value properties are not yet tested, feel free to enhance the test case.

The problem you had with multi-value properties probably was that commons-fileupload's FileItem only has access methods for single-value String parameters. You have to get the multi-value ones from the ServletRequest yourself.

Regarding UTF-8: The test case uses the UTF-8 encoded log4j.xml from src/test-resources for its multipart request without problems. But maybe this isn't comprehensive enough.

Comment by Fabrizio Giustina [ 24/Sep/05 ]

The new filter is now in svn, but it still can't replace the COS based one because it actually doesn't support multi-value parameters.
The test case has been ehanced with multivalue and utf8 parameters (the problem was in encoding of parameters, not files) and it tests both (cos and commons file upload) filters... when everything will work using the new filter we will replace the old one and remove the oreilly dependency.

Comment by Philipp Bracher [ 17/May/06 ]

We should fix this in the 3.0 relese

Comment by Philipp Bracher [ 13/Jun/06 ]

postponed again

Comment by Fabrizio Giustina [ 26/Sep/07 ]

Setting release version and moving to an higher priority, cos license terms could be problematic and this issue is here from too much time...

After my recent fixes the new commons-fileupload implementation looks good, and someway also better than the cos based one (there is an additional request wrapper which makes easier to deal with multipart forms).

I'd like to see this becoming the default in 3.1-m4, by renaming the old cos filter to CosMultipartRequestFilter and CommonsFileUploadMultipartRequestFilter to MultipartRequestFilter. If no issues are found in this milestone we can easily drop the cos dependency (along with the filter implementation) for the first RC.

Comment by Sameer Charles [ 26/Sep/07 ]

that's great, but just wanted to let you guys know that magnolia has a permission to redistribute cos library without any restrictions.
We attain this license sometime last year.

Comment by Fabrizio Giustina [ 30/Sep/07 ]

> that's great, but just wanted to let you guys know that magnolia has a permission to redistribute cos library without any restrictions.
> We attain this license sometime last year.

well, that's great (unfortunately looks like nobody was aware of that), but anyway switching to commons-fileupload is probably still an improvement.
I don't know if the granted license of cos is compatible with *gpl (may be) and if it has been granted only for the redistribution of magnolia it will still be a problem for anybody who develops anything based on magnolia (is the "unlimited use without buying the book" transitive?).

Anyway we now have the alternative implementation with commons-fileupload fully working, and commons-fileupload is apache2, it couldn't be better.
The switch of the default implementation has been committed to svn, note that it should not cause any problem on upgrades since I did an "hard" switch, renaming the commons-fileupload implementation with the same name of the previous cos-based one)

Comment by Magnolia International [ 02/Nov/07 ]

I'm sorry but I'm going to have to reopen this. At least temporarily, I'll switch back the default to the CosMultipartRequestFilter (not renaming classes).

The problem is that with the commons-upload-based implementation, request parameters are lost. I haven't examined the problem deeply, and it might be that it's only because in my case, they were part of the query string (ie params in the URLs were lost, but the file was uploaded properly) I'm not sure that's spec compliant (having both url and post params) but in the case of a file upload through the fck editor, it breaks everything and we can't afford that just now.

Another problem (which I'll try to fix now) is that the (commons-upload) MultipartRequestFilter is swallowing all exceptions that happen down the filter chain, which is just plain wrong (I'll fix that). Maybe we could introduce an AbstractMRF, because in the end all those filters (should) do is wrap the request and pass it on. This would ensure consistent behavior betweem the two.

Comment by Fabrizio Giustina [ 02/Nov/07 ]

> The problem is that with the commons-upload-based implementation, request parameters are lost.

mh, not sure if mixing query string with a multipart form parameters could be a standard behavior, but if request.getParameter() actually returns query string parameters also in case of multiparts this can be easily fixed in the commons-fileupload filter (just adding to the parameters from the query string to the ones in the multipart form).

Comment by Fabrizio Giustina [ 16/Nov/07 ]

Grégory, can you explain exactly how to reproduce the problem you saw? Can we put together a testcase for it?

Comment by Magnolia International [ 16/Nov/07 ]

Hmm, right now I don't remember exactly which one, but I had the issue with some dialog. I don't think it even had any upload, but it's <form> action parameter add URL parameters, and its method was POST. The URLs parameters were lost along the way.

I'll try to reproduce... after the weekend

Comment by Jan Haderka [ 04/Sep/08 ]

Since we have license to redistribute, it is not system critical and we have some time to solve all the issues with commons-fileupload before making the switch.

Generated at Mon Feb 12 03:18:26 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.