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 Kotlin filter parameter bug in Router DSLs #26838

Closed
ShindongLee opened this issue Apr 21, 2021 · 2 comments
Closed

Fix Kotlin filter parameter bug in Router DSLs #26838

ShindongLee opened this issue Apr 21, 2021 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@ShindongLee
Copy link
Contributor

ShindongLee commented Apr 21, 2021

fun filter(filterFunction: (ServerRequest, (ServerRequest) -> ServerResponse) -> ServerResponse) {
builder.filter { request, next ->
filterFunction(request) {
next.handle(request)
}
}

fun filter(filterFunction: suspend (ServerRequest, suspend (ServerRequest) -> ServerResponse) -> ServerResponse) {
builder.filter { serverRequest, handlerFunction ->
mono(Dispatchers.Unconfined) {
filterFunction(serverRequest) {
handlerFunction.handle(serverRequest).awaitSingle()
}
}
}
}

  1. Function filterFunction takes serverRequest and function handler as parameters. (I just name it second parameter of filterFunction as handler.)
  2. Function handler also takes serverRequest as parameter.
  3. These DSLs define handler.
  4. Function handlers that these DSLs defined are now using mistakenly filterFunction's serverRequest, not their own serverRequest parameters, which makes their own parameters meaningless.

This bug only exists in Kotlin DSL.

You can see java codes have no problem as follows.

R filter(ServerRequest request, HandlerFunction<T> next) throws Exception;

In java you can override filter something like

ServerRequest newRequest = someOperation(request)
return next.handle(newRequest)

but in Kotlin DSL, even if you write

filter { serverRequest, handlerFunction ->
    val newServerRequest = someOperation(serverRequest)
    handlerFunction(newServerRequest)
}

handlerFunction will use original serverRequest, not the newServerRequest

@ShindongLee ShindongLee changed the title Router handlerFunction in filter should use its parameter (request) Router handlerFunction in filter should use its own parameter (serverRequest) Apr 21, 2021
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 21, 2021
@ShindongLee ShindongLee changed the title Router handlerFunction in filter should use its own parameter (serverRequest) [KotlinDSL][Bug] handler (second parameter of filterFunction) in [Co]RouterFunctionDsl is using wrong parameter (not its own) Apr 24, 2021
@ShindongLee ShindongLee changed the title [KotlinDSL][Bug] handler (second parameter of filterFunction) in [Co]RouterFunctionDsl is using wrong parameter (not its own) [KotlinDSL][Bug] handler (second parameter of filterFunction) that [Co]RouterFunctionDsl defines is using wrong parameter (not its own) Apr 24, 2021
@sdeleuze sdeleuze self-assigned this Apr 28, 2021
@sdeleuze sdeleuze added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 28, 2021
@sdeleuze sdeleuze added this to the 5.3.7 milestone Apr 28, 2021
@sdeleuze
Copy link
Contributor

Good catch, to be backported on 5.2.x as well. I will be able to merge the proposed fix with additional tests asked here, thanks.

@snicoll snicoll changed the title [KotlinDSL][Bug] handler (second parameter of filterFunction) that [Co]RouterFunctionDsl defines is using wrong parameter (not its own) HandlerFunction (second parameter of filterFunction) that [Co]RouterFunctionDsl defines is using wrong parameter (not its own) Apr 28, 2021
@ShindongLee
Copy link
Contributor Author

ShindongLee commented Apr 28, 2021

@sdeleuze I added tests. Thank you !

@sdeleuze sdeleuze changed the title HandlerFunction (second parameter of filterFunction) that [Co]RouterFunctionDsl defines is using wrong parameter (not its own) Fix Kotlin filter parameter bug in Router DSLs May 10, 2021
@sdeleuze sdeleuze added the for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x label May 10, 2021
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x labels May 10, 2021
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this issue Aug 20, 2021
Co-authored-by: hojongs <hojong.jjh@gmail.com>
Co-authored-by: bjh970913 <bjh970913@gmail.com>

Closes spring-projectsgh-26838
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this issue Mar 26, 2022
Co-authored-by: hojongs <hojong.jjh@gmail.com>
Co-authored-by: bjh970913 <bjh970913@gmail.com>

Closes spring-projectsgh-26838
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
3 participants