@RequestBody argument resolver throws 'java.io.IOException: Stream closed'

[kerkhofsd]

I'm submitting a ... (check one with "x")

[x] bug report
[ ] feature request
[ ] question

Expected Behavior

No IOException when working with the @RequestBody parameter

Current Behavior

Webscript method arguments annotated with @RequestBody are resolved with the RequestBodyArgumentResolver. This resolver reads the input stream of the request and tries to parse it into the requested type. Nevertheless if the input stream has already been read (and closed), this will throw an IOException.

Possible Solution

Use content.getContent() instead of content.getInputStream() to resolve the argument. This will not throw an exception if the input streams is already closed, and will just return the content that was read earlier.

Steps to Reproduce (for bugs)

1. Create an @Before webscript handler that has e.g. the Content as an argument
2. Create a webscript that uses the @RequestBody annotation

Your Environment

• Alfresco version used: 5.2.4
• DE version used: 1.7.6

[vierbergenlars]

Please take into account that using content.getContent() will allocate a very large string for webscripts to which files are uploaded. Binary files that are not valid UTF-16 will be corrupted.

Consider using content.getSize() to detect if the body is empty instead. (I am guessing the exception is thrown from here: https://github.com/xenit-eu/dynamic-extensions-for-alfresco/blob/master/annotations-runtime/shared/src/main/java/com/github/dynamicextensionsalfresco/webscripts/arguments/RequestBodyArgumentResolver.java#L71)

[kerkhofsd]

@vierbergenlars

1. AFAIK the @RequestBody parameter should not be used to process e.g. file uploads. I think we can assume the body processed by the @RequestBody handler will not be that large?
2. The getContent() method implementation in the InputStreamContent class takes the encoding into account:

    public String getContent()
        throws IOException
    {
        // ensure we only try to read the content once - as this method may be called several times
        // but the inputstream can only be processed a single time
        if (this.content == null)
        {
            ByteArrayOutputStream os = new ByteArrayOutputStream(1024);
            FileCopyUtils.copy(stream, os);  // both streams are closed
            byte[] bytes = os.toByteArray();
            // get the encoding for the string
            String encoding = getEncoding();
            // create the string from the byte[] using encoding if necessary
            this.content = (encoding == null) ? new String(bytes) : new String(bytes, encoding);
        }
        return this.content;
    }

3. InputStreamContent#getSize() always returns -1, hence we cannot use this method to determine if the body is empty.

Nevertheless always reading the content to a String indeed seems bad practice.
Maybe the root problem of this issue is that the input stream is already closed and not that we try to access it twice.
Will do some more investigation for a feasible solution.

[todorinskiz]

If the endpoint is supposed to process large payloads such as binaries (>100MB), it's better done on input stream level, there is no reason to wrap a binary file as a POJO, thus in that scenario @RequestBody makes no sense to use.
That being said, I think it's safe to buffer the value of the @RequestBody.

[vierbergenlars]

I think we can assume the body processed by the @RequestBody handler will not be that large?

You can indeed assume that the body that the user wants to process with a @RequestBody handler will not be that large.
However, you also have the case of a malicious client that sends you a 10 GB garbage body, which will then seriously stress/OOM the Alfresco server when it is loaded into memory.

In your code snippets, you have 2 potentially large allocations: byte[] bytes and the String that is created from those bytes.

Additionally, the cached content is limited to 1024 bytes, which is likely a bit too small for things like alfred api, where the search and metadata endpoint both can take much larger structures (search query and all metadata of a document)

[todorinskiz]

However, you also have the case of a malicious client that sends you a 10 GB garbage body, which will then seriously stress/OOM the Alfresco server when it is loaded into memory.

That is not the right place to fix that problem? You already have such protection at the level of Tomcat configuration.
server.xml
<Connector port="80" protocol="HTTP/1.1" connectionTimeout="20000" redirectPort="8443" maxPostSize="67589953" />