[jira] [Commented] (SLING-5948) Support Streaming uploads.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15929727#comment-15929727
 ] 

ASF GitHub Bot commented on SLING-5948:
---

Github user ieb closed the pull request at:

https://github.com/apache/sling/pull/161


> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
>  Labels: Sling-9-ReleaseNotes
> Fix For: Servlets Post 2.3.14, Engine 2.6.0
>
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15449000#comment-15449000
 ] 

Ian Boston commented on SLING-5948:
---

The way in which the file name is detected my be wrong. It delegates to the 
Commons File Upload FileItemStream.getFieldName() as was indicated in various 
examples, however, the JavaDoc at 
https://commons.apache.org/proper/commons-fileupload/apidocs/org/apache/commons/fileupload/FileItemStream.html#getName--
 indicates that the getName method should be used. I have opened an new issue 
SLING-6019 to fix this. 

We should not implement header parsing in the Sling code unless the code in 
FileUpload contains a bug.

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
>  Labels: Sling-9-ReleaseNotes
> Fix For: Servlets Post 2.3.14, Engine 2.6.0
>
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Satya Deep Maheshwari (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15448766#comment-15448766
 ] 

Satya Deep Maheshwari commented on SLING-5948:
--

[~ianeboston], while trying this out, I am seeing that the form fields that I 
am sending are getting detected as file which is to be streamed. For e.g. , 
here's the curl command that I am using:

{code}
curl -o /dev/null -v -F key1=value1 -F file=@file.txt -w 
%{time_connect}:%{time_starttransfer}:%{time_total} 
http://admin:admin@localhost:8080/temp/file4?uploadmode=stream
{code}

While trying to debug this, I see that on the server, at (1), the part 
containing {{key1=value1}} gets detected as a file. This seems to be because of 
the way {{RequestPartsIterator.getSubmittedFileName}} is implemented at (2),  

Should it be implemented such that it specifically searches for the 
{{filename}} header for returning the submitted filename?The multipart rfc at 
(3) indicates that {{filename}} is a legitimate header for the part containing 
the file in a multipart request. Roughly, can we implement it as below?

{code}
//Taken from https://java.net/jira/browse/SERVLET_SPEC-57
   public String getSubmittedFileName() {
String header = this.getHeader("content-disposition");
if(header == null)
return null;
for(String headerPart : header.split(";"))
{
if(headerPart.trim().startsWith("filename"))
{
return headerPart.substring(headerPart.indexOf('=') + 
1).trim()
.replace("\"", "");
}
}
return null;
}
{code}

cc [~shgu...@adobe.com]
(1) - 
https://github.com/apache/sling/blob/trunk/bundles/servlets/post/src/main/java/org/apache/sling/servlets/post/impl/operations/StreamedUploadOperation.java#L95
(2) - 
https://github.com/apache/sling/blob/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/parameters/RequestPartsIterator.java#L148
(3) - https://www.ietf.org/rfc/rfc2388.txt

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
>  Labels: Sling-9-ReleaseNotes
> Fix For: Servlets Post 2.3.14, Engine 2.6.0
>
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15418729#comment-15418729
 ] 

Ian Boston commented on SLING-5948:
---

JDK8 builds passed. JDK7 builds failed at launchpad, as it requires JDK8. Will 
commit.

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on Servlet 3 to get the Part API, 
> requiring some patches to the existing test classes.
> To support both 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15418645#comment-15418645
 ] 

ASF GitHub Bot commented on SLING-5948:
---

GitHub user ieb opened a pull request:

https://github.com/apache/sling/pull/161

SLING-5948 Support Streaming.

Creating pull request to trigger builds by CI and Travis. See issue for 
details. Will commit to SVN if all ok.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ieb/sling SLING-5948-P2

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/sling/pull/161.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #161


commit b1409a08ac8aba80a41a4588b180fbf8f49c3319
Author: ieb 
Date:   2016-08-08T09:11:13Z

SLING-5948 Support Streaming - Implementation of proposal 1 using a 
Interator

commit 870dd74f8f752e27bad2d0c4dd6937573342b941
Author: ieb 
Date:   2016-08-09T16:15:23Z

SLING-5948 Support Streaming - Implementation of StreamingUploadOperation 
to work with proposal 2 implemented in previous commit (Interator)

commit b15bc88e9058d4944b9755a5d1493b7ee6b6b547
Author: ieb 
Date:   2016-08-11T14:35:18Z

SLING-5948 Support Streaming - Added unit test coverage and tested in 
launchpad

commit 52e09abbc9b9fd23cc010b790e328a64bc554c78
Author: ieb 
Date:   2016-08-12T09:52:39Z

SLING-5948 Support Streaming - Cleaned up headers, added more javadoc, 
incorporated feedback from RTC

commit 136555a9b387e739aebbd76e36d8d8bdae1ab217
Author: ieb 
Date:   2016-08-12T10:06:15Z

SLING-5948 Support Streaming - fixed imports and formatting

commit 0b427bceb0b7396f0563f1b51d43a5463b9bb2a8
Author: ieb 
Date:   2016-08-12T10:08:52Z

SLING-5948 Support Streaming - removed default javadoc on test classes




> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15418641#comment-15418641
 ] 

Ian Boston commented on SLING-5948:
---

Patch in Git has been updated with comments so far and some cleanup. 
https://github.com/apache/sling/compare/trunk...ieb:SLING-5948-P2?expand=1

Doing a full build prior and final testing to committing.

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15418449#comment-15418449
 ] 

Ian Boston commented on SLING-5948:
---

Agreed, thank you for the rfc reference. Header is now {{Sling-uploadmode}}, 
patch update and git commit pending for this change.

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on Servlet 3 to get the Part API, 
> requiring some patches to the 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Robert Munteanu (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15418432#comment-15418432
 ] 

Robert Munteanu commented on SLING-5948:


I suggest dropping the 'X-' prefix, see https://tools.ietf.org/html/rfc6648

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on Servlet 3 to get the Part API, 
> requiring some patches to the existing test classes.
> To support both 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15417521#comment-15417521
 ] 

Ian Boston commented on SLING-5948:
---

{{X-sling-uploadmode}} yes, no problem. 

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on Servlet 3 to get the Part API, 
> requiring some patches to the existing test classes.
> To support both methods a standard Servlet to handle streamed 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Bertrand Delacretaz (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15417510#comment-15417510
 ] 

Bertrand Delacretaz commented on SLING-5948:


I just had a quick look and the performance improvements are impressive, thanks 
for this!

A nitpick: could the header be named {{X-sling-uploadmode}} to be more specific?

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15411962#comment-15411962
 ] 

Ian Boston commented on SLING-5948:
---

Implementing the upload as Sling Operation and supporting the full Sling 
protocol as expressed in the ModifyOperation results in a significant amount of 
cut and paste coding from the Modify Operation as all the calls in the 
ModifyOperation expect to see Sling RequestProperty(s) in a Map, hence the 
class hierarchy can't be extended to work with Parts, unless 80% of the methods 
are refactored/duplicated. To avoid creating duplicate code to support, I will 
implement an UploadOperation that only supports upload. If a user wants to 
perform a non standard upload, they should create the target Resource in 
advance of the upload. Although this might be an overhead, streamed uploads are 
generally going to take time to complete so the overhead is probably acceptable.

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine
>Affects Versions: Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2.patch
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs custom servlets that understand how to process the Iterator
> * Probably cant use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would