Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix endpoints with HttpServletResponse parameters causing empty produces #36

Closed
uuf6429 opened this issue May 10, 2019 · 1 comment · Fixed by #39
Closed

Fix endpoints with HttpServletResponse parameters causing empty produces #36

uuf6429 opened this issue May 10, 2019 · 1 comment · Fixed by #39
Assignees
Labels
Bug Something isn't working Spring Relates to the SpringConverter
Milestone

Comments

@uuf6429
Copy link
Contributor

uuf6429 commented May 10, 2019

Setup information

hikaku version: 2.2.0
specification converter: OpenApi
implementation converter: Spring
build tool and version: gradle 5, kotlin 1.3.21
test framework: junit 5.1

Describe the bug

Modifying response passed as method parameter (and thus not requiring a method return type) causes Hikaku to ignore produces in @XXXMapping annotation.

Expected behavior

Hikaku Spring converter should probably check that result is not void and there is no parameter of type HttpServletResponse.

Code samples

The following PoC produces this issue:

    @GetMapping("v1/test", produces = ["application/pdf"])
    fun invoke(response: HttpServletResponse)
    {
        try {
            val inputStream = fileStreamer.stream("s3://some-endpoint/test.pdf")
            IoUtils.copy(inputStream, response.outputStream)
        } catch (ex: NoSuchKeyException) {
            response.status = 404
        }
    }

The root cause is the this.value.method.hasNoReturnType() part here:
https://github.com/codecentric/hikaku/blob/master/spring/src/main/kotlin/de/codecentric/hikaku/converters/spring/extensions/ProducesSpringExtension.kt#L20

    if (isNotErrorPath && ((hasNoResponseBodyAnnotation && hasNoRestControllerAnnotation) || this.value.method.hasNoReturnType())) {
        return emptySet()
    }

Potential solution

Off the top of my mind, this might fix the problem:

    val isNotErrorPath = !this.key.patternsCondition.patterns.contains("/error")
    val hasNoResponseBodyAnnotation = !this.value.providesResponseBodyAnnotation()
    val hasNoRestControllerAnnotation = !this.value.providesRestControllerAnnotation()
    val hasHttpServletResponseParam = this.value.hasHttpServletResponseParam()

    if (isNotErrorPath && (hasNoResponseBodyAnnotation && hasNoRestControllerAnnotation)) {
        return emptySet()
    }

    if (isNotErrorPath && (this.value.method.hasNoReturnType() && !hasHttpServletResponseParam)) {
        return emptySet()
    }

    // ...

    private fun HandlerMethod.hasHttpServletResponseParam() = this.methodParameters
        .any { it.parameterType.isAssignableFrom(HttpServletResponse::class.java) }
@uuf6429 uuf6429 added the Bug Something isn't working label May 10, 2019
@cc-jhr cc-jhr added the Spring Relates to the SpringConverter label May 13, 2019
@lmller
Copy link
Contributor

lmller commented May 18, 2019

Hey, thanks for reaching out! If you like, you can provide a PR (with unit tests) :-) . Otherwise, I'll take a look at it some time later.

@cc-jhr cc-jhr added this to the v2.2.1 milestone Jun 2, 2019
@uuf6429 uuf6429 changed the title Endpoints with HttpServletResponse parameters do not need to return anything and Hikaku misses this scenario Fix endpoints with HttpServletResponse parameters causing empty produces Jun 8, 2019
cc-jhr pushed a commit that referenced this issue Jun 17, 2019
…#39)

* Handle javax.servlet.http.HttpServletResponse with tests
* Updated documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Spring Relates to the SpringConverter
Projects
None yet
3 participants