-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Send REST response for incorrectly encoded url params #28909
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,8 @@ public abstract class RestRequest implements ToXContent.Params { | |
| private final Set<String> consumedParams = new HashSet<>(); | ||
| private final SetOnce<XContentType> xContentType = new SetOnce<>(); | ||
|
|
||
| private Exception exception; | ||
|
|
||
| /** | ||
| * Creates a new RestRequest | ||
| * @param xContentRegistry the xContentRegistry to use when parsing XContent | ||
|
|
@@ -78,7 +80,11 @@ public RestRequest(NamedXContentRegistry xContentRegistry, String uri, Map<Strin | |
| this.rawPath = uri; | ||
| } else { | ||
| this.rawPath = uri.substring(0, pathEndPos); | ||
| RestUtils.decodeQueryString(uri, pathEndPos + 1, params); | ||
| try { | ||
| RestUtils.decodeQueryString(uri, pathEndPos + 1, params); | ||
| } catch (Exception e) { | ||
| this.exception = e; | ||
| } | ||
| } | ||
| this.params = params; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change still bothers me a lot. Again, I appreciate the motivation for it but the implementation that we are being forced into here leaves a lot to be desired (I am not faulting you for it, we are somewhat painted into a corner here). Another problem here is that now the object is a garbage state, we have no idea what the state of |
||
| this.headers = Collections.unmodifiableMap(headers); | ||
|
|
@@ -206,6 +212,10 @@ public SocketAddress getLocalAddress() { | |
| return null; | ||
| } | ||
|
|
||
| public Exception getException() { | ||
| return exception; | ||
| } | ||
|
|
||
| public final boolean hasParam(String key) { | ||
| return params.containsKey(key); | ||
| } | ||
|
|
||
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.
Adding this mutable field (and thus making
RestRequestnot immutable) is not attractive.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.
@jasontedor thanks!
Have another look ?