-
Notifications
You must be signed in to change notification settings - Fork 34
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
UNDERTOW-1581: Fix RoutingHandler allMethodsMatcher validation #71
base: main
Are you sure you want to change the base?
Conversation
The flake |
Updates the allMethodsMatcher to lookup templates using the match method instead of get in order to normalize the input. Previously registration would throw an exception when routes were added for the same path template with different parameter names despite being equivalent. From undertow-io/undertow#799
1d4ba4a
to
2d6cdd9
Compare
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs |
---|---|---|---|---|---|
✔️ | Build - JDK 11 | Logs | Raw logs | ||
✖ | Build - JDK 17 | Build with Maven |
Failures | Logs | Raw logs |
Full information is available in the Build summary check run.
Failures
⚙️ Build - JDK 17 #
- Failing: servlet
! Skipped: websocket websocket/core websocket/servlet and 1 more
📦 servlet
✖ io.undertow.servlet.test.dispatcher.DispatcherForwardTestCase.testPathBasedInclude
line 137
- More details - Source on GitHub
org.junit.ComparisonFailure: expected:<...etContext/dispatch /[dispatch]> but was:<...etContext/dispatch /[forward]>
at org.junit.Assert.assertEquals(Assert.java:117)
at org.junit.Assert.assertEquals(Assert.java:146)
at io.undertow.servlet.test.dispatcher.DispatcherForwardTestCase.testPathBasedInclude(DispatcherForwardTestCase.java:137)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
.add(HttpMethodNames.POST, "/foo/{baz}", new HttpHandler() { | ||
@Override | ||
public void handleRequest(HttpServerExchange exchange) throws Exception { | ||
exchange.writeAsync("foo-path" + exchange.getQueryParameters().get("bar")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it should be like in https://github.com/undertow-io/undertow/pull/799/files#diff-b365caf66586093f040e0a2915a095a4bf1f4ed87343a31998507406f358e827R132?
exchange.writeAsync("foo-path" + exchange.getQueryParameters().get("bar")); | |
exchange.getResponseSender().send("foo-path" + exchange.getQueryParameters().get("bar")); |
Updates the allMethodsMatcher to lookup templates using the
match method instead of get in order to normalize the input.
Previously registration would throw an exception when routes
were added for the same path template with different parameter names
despite being equivalent.
From undertow-io/undertow#799