Skip to content
This repository has been archived by the owner on Jul 2, 2023. It is now read-only.

Fix issue with submitting multipart data #33

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions jaxrs-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ dependencies {
compile('javax.ws.rs:jsr311-api:1.1.1') {
exclude module: 'junit'
}
compile group: 'commons-io', name: 'commons-io', version: '2.4'

}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import org.grails.plugins.jaxrs.core.JaxrsContext
import org.grails.plugins.jaxrs.core.JaxrsFilter
import org.grails.plugins.jaxrs.core.JaxrsListener
import org.grails.plugins.jaxrs.core.JaxrsUtil
import org.grails.plugins.jaxrs.filters.ReusableHttpRequestStreamFilter
import org.grails.plugins.jaxrs.provider.*
import org.springframework.boot.context.embedded.FilterRegistrationBean
import org.springframework.boot.context.embedded.ServletListenerRegistrationBean
Expand All @@ -23,6 +24,7 @@ class JaxrsCoreGrailsPlugin extends Plugin {
/**
* Load order.
*/
def loadBefore = ['core']
def loadAfter = ['hibernate3', 'hibernate4', 'hibernate5', 'controllers', 'services', 'spring-security-core']

/**
Expand Down Expand Up @@ -138,6 +140,11 @@ mechanism for implementing RESTful web services.
bean.autowire = true
}
}
reusableHttpRequestStreamFilter(FilterRegistrationBean) {
filter = bean(ReusableHttpRequestStreamFilter)
urlPatterns = ['/*']
order = Ordered.HIGHEST_PRECEDENCE
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2009 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.grails.plugins.jaxrs.filters

import groovy.transform.CompileStatic
import javax.servlet.FilterChain
import javax.servlet.ServletException
import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletResponse
import org.springframework.web.filter.OncePerRequestFilter
import org.grails.plugins.jaxrs.servlet.http.ReusableStreamHttpServletRequestWrapper

/**
*
* @author Alex Stoia
*/
@CompileStatic
class ReusableHttpRequestStreamFilter extends OncePerRequestFilter {

/**
*
*/
@Override
protected void doFilterInternal(HttpServletRequest request,
HttpServletResponse response, FilterChain chain) throws ServletException,
IOException {
chain.doFilter(new ReusableStreamHttpServletRequestWrapper(request), response)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright 2009 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.grails.plugins.jaxrs.servlet

import groovy.transform.CompileStatic
import javax.servlet.ServletInputStream
import javax.servlet.ReadListener


/**
*
* @author Alex Stoia
*/
@CompileStatic
class ReusableServletInputStream extends ServletInputStream {
private BufferedInputStream wrappedStream
private ServletInputStream originalStream

ReusableServletInputStream(ServletInputStream originalStream) {
super()
this.originalStream = originalStream
this.wrappedStream = new BufferedInputStream(originalStream)
}


@Override
boolean isReady() {
return originalStream.isReady()
}
@Override
boolean isFinished() {
return originalStream.isFinished()
}
@Override
void setReadListener(ReadListener readListener) {
originalStream.setReadListener(readListener)
}

@Override
public int read() throws IOException {
return wrappedStream.read()
}
@Override
boolean markSupported() {
return wrappedStream.markSupported()
}
@Override
void mark(int readlimit) {
wrappedStream.mark(readlimit)
}
@Override
void reset() {
wrappedStream.reset()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright 2009 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.grails.plugins.jaxrs.servlet.http

import groovy.transform.CompileStatic
import javax.servlet.ServletInputStream
import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletRequestWrapper
import org.apache.commons.io.IOUtils
import org.grails.plugins.jaxrs.servlet.ReusableServletInputStream

/**
*
* @author Alex Stoia
*/
@CompileStatic
class ReusableStreamHttpServletRequestWrapper extends HttpServletRequestWrapper {
private BufferedInputStream wrappedStream
private ServletInputStream usedStream

ReusableStreamHttpServletRequestWrapper(HttpServletRequest request) {
super(request)
ServletInputStream originalStream = request.getInputStream()
usedStream = new ReusableServletInputStream(originalStream)
usedStream.mark(Integer.MAX_VALUE)
// read the stream contents, otherwise this will not be available in
// the getInputStream method. Also tried reading one byte, works on
// first request, fails on all subsequent ones. Got no idea why at
// the moment
IOUtils.toString(usedStream, 'UTF-8')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is concerning to me because this ultimately will require all payloads to be read into memory for the duration of the request. This doesn't allow authors to efficiently pipe streams together, and can potentially cause large memory requirements, depending on the expected payload sizes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Bud.

Thanks for your review. This fix immediately unblocked me for using multipart requests in jersey, though I understand the problem. Will have a think on how to improve this

How did you get it to work for issue 21?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Bud.

I think I found a better solution which involves overwriting the MultiPartReaderServerSide to support the Grails requests. My question is, you think it is ok if jersey-multipart will be added as a dependency into the grails-jaxrs-jersey1 plugin? I see no reason not to.

Will push the new fix soon

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking a look at these PR's. Did this ever get the multipart library added?

}

@Override
public ServletInputStream getInputStream() {
usedStream.reset()
return usedStream
}
}
4 changes: 4 additions & 0 deletions jaxrs-example/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ apply plugin:"asset-pipeline"
ext {
grailsVersion = project.grailsVersion
gradleWrapperVersion = project.gradleWrapperVersion
jerseyVersion = '1.19'
}

repositories {
Expand Down Expand Up @@ -64,6 +65,9 @@ dependencies {
compile project(':jaxrs-jersey1')
compile project(':jaxrs-swagger-ui')
testCompile project(':jaxrs-integration-test')

compile "com.sun.jersey.contribs:jersey-multipart:$jerseyVersion"
testCompile 'org.apache.httpcomponents:httpmime:4.4'
}

task wrapper(type: Wrapper) {
Expand Down
4 changes: 3 additions & 1 deletion jaxrs-example/grails-app/conf/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ dataSource:
driverClassName: org.h2.Driver
username: sa
password:

environments:
development:
dataSource:
dbCreate: create-drop
url: jdbc:h2:mem:devDb;MVCC=TRUE;LOCK_TIMEOUT=10000;DB_CLOSE_ON_EXIT=FALSE
test:
server:
port: 0
dataSource:
dbCreate: update
url: jdbc:h2:mem:testDb;MVCC=TRUE;LOCK_TIMEOUT=10000;DB_CLOSE_ON_EXIT=FALSE
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.budjb
import io.swagger.annotations.Api

import javax.ws.rs.POST
import javax.ws.rs.Path
import javax.ws.rs.Consumes
import javax.ws.rs.Produces
import javax.ws.rs.core.Response
import javax.ws.rs.core.MediaType

import com.sun.jersey.multipart.FormDataParam
import com.sun.jersey.multipart.FormDataBodyPart

import grails.converters.JSON
@Path('/api/testMultipart')
@Api('test')
class TestMultipartResource {
@POST
@Consumes(MediaType.MULTIPART_FORM_DATA )
@Produces(MediaType.APPLICATION_JSON )
@Path('/upload')
Response getTestMultipartRepresentation(
@FormDataParam('file') File file,
@FormDataParam('file') FormDataBodyPart fileBodyPart) {
// try with
// curl -v -X POST -H "Content-Type: multipart/form-data" -F "file=@src/integration-test/groovy/com/budjb/resources/pdf-sample.pdf" http://localhost:8060/api/testMultipart/upload
JSON ret = [
name: fileBodyPart.getContentDisposition().getFileName(),
size: file.size(),
mimeType: fileBodyPart.getMediaType().toString()
] as JSON
return Response.status(200).entity(ret).build()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.budjb

import org.apache.http.entity.mime.MultipartEntityBuilder
import org.apache.http.entity.ContentType
import grails.test.mixin.integration.Integration
import org.grails.plugins.jaxrs.test.JaxrsIntegrationSpec
import org.grails.plugins.jaxrs.test.JaxrsRequestProperties

@Integration
class TestMultipartResourceSpec extends JaxrsIntegrationSpec {
def 'Ensure can send a multipart file to the resource'() {
given: "A file to send"
File file = new File('src/integration-test/groovy/com/budjb/resources/pdf-sample.pdf')

when: "Sending a file to the web service"
def response = makeRequest(new JaxrsRequestProperties(method: 'POST',
uri: '/api/testMultipart/upload',
headers: ['Content-Type' : ['multipart/form-data; boundary=fnord']],
body: getMultipartBody(file)))
def result = new groovy.json.JsonSlurper().parseText(response.bodyAsString)

then: "The response is correct"
200 == response.status
'pdf-sample.pdf' == result.name
result.size > 0
'application/pdf' == result.mimeType
}
/**
* Return the list of additional resources to build the JAX-RS servlet with.
*
* @return
*/
@Override
List getResources() {
return []
}


private byte[] getMultipartBody(File file) {
MultipartEntityBuilder e = new MultipartEntityBuilder()
e.setBoundary("fnord")
e.addBinaryBody("file", file, ContentType.create("application/pdf"), file.name)
ByteArrayOutputStream baos = new ByteArrayOutputStream()
e.build().writeTo(baos)
return baos.toByteArray()
}
}
Binary file not shown.