[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[
https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16497125#comment-16497125
]
ASF GitHub Bot commented on CAMEL-11162:
aldettinger closed pull request #2355: CAMEL-11162: Corrected doc and
alleviated tests
URL: https://github.com/apache/camel/pull/2355
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/camel-core/src/main/docs/rest-dsl.adoc
b/camel-core/src/main/docs/rest-dsl.adoc
index 7402ba94a59..6044de1edbc 100644
--- a/camel-core/src/main/docs/rest-dsl.adoc
+++ b/camel-core/src/main/docs/rest-dsl.adoc
@@ -652,7 +652,7 @@ functionality is only applicable for query parameters.
From *Camel 2.22* onwards its possible to enable validation of the incoming
client request.
The validation checks for the following:
-- Content-Type header matches what the Rest DSL consumes. (Returns HTTP Status
416)
+- Content-Type header matches what the Rest DSL consumes. (Returns HTTP Status
415)
- Accept header matches what the Rest DSL produces. (Returns HTTP Status 406)
- Missing required data (query parameters, HTTP headers, body). (Returns HTTP
Status 400)
diff --git
a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyAcceptTest.java
b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyAcceptTest.java
index d48472ef7e0..4b9927883c8 100644
---
a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyAcceptTest.java
+++
b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyAcceptTest.java
@@ -80,7 +80,6 @@ public void configure() throws Exception {
rest("/users/")
.post("{id}/update").consumes("application/json").produces("application/json")
.route()
-.to("mock:input")
.setBody(constant("{ \"status\": \"ok\" }"));
}
};
diff --git
a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyContentTypeTest.java
b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyContentTypeTest.java
index b4d50bc01f2..4076b0cd905 100644
---
a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyContentTypeTest.java
+++
b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyContentTypeTest.java
@@ -78,7 +78,6 @@ public void configure() throws Exception {
rest("/users/")
.post("{id}/update").consumes("application/json").produces("application/json")
.route()
-.to("mock:input")
.setBody(constant("{ \"status\": \"ok\" }"));
}
};
diff --git
a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyRequiredBodyTest.java
b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyRequiredBodyTest.java
index 3ef0ebb0541..6c338e96cec 100644
---
a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyRequiredBodyTest.java
+++
b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyRequiredBodyTest.java
@@ -70,7 +70,6 @@ public void configure() throws Exception {
.post("{id}/update").consumes("application/json").produces("application/json")
.param().name("body").required(true).type(RestParamType.body).endParam()
.route()
-.to("mock:input")
.setBody(constant("{ \"status\": \"ok\" }"));
}
};
diff --git
a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyRequiredHttpHeaderTest.java
b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyRequiredHttpHeaderTest.java
index 728826eeace..61222b79743 100644
---
a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyRequiredHttpHeaderTest.java
+++
b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyRequiredHttpHeaderTest.java
@@ -72,7 +72,6 @@ public void configure() throws Exception {
.post("{id}/update").consumes("application/json").produces("application/json")
.param().name("country").required(true).type(RestParamType.header).endParam()
.route()
-.to("mock:input")
.setBody(constant("{ \"status\": \"ok\"
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16497126#comment-16497126 ] ASF GitHub Bot commented on CAMEL-11162: Github user aldettinger closed the pull request at: https://github.com/apache/camel/pull/2355 > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496945#comment-16496945 ] ASF GitHub Bot commented on CAMEL-11162: aldettinger commented on a change in pull request #2355: CAMEL-11162: Corrected doc and alleviated tests URL: https://github.com/apache/camel/pull/2355#discussion_r192188153 ## File path: camel-core/src/main/docs/rest-dsl.adoc ## @@ -652,7 +652,7 @@ functionality is only applicable for query parameters. From *Camel 2.22* onwards its possible to enable validation of the incoming client request. The validation checks for the following: -- Content-Type header matches what the Rest DSL consumes. (Returns HTTP Status 416) +- Content-Type header matches what the Rest DSL consumes. (Returns HTTP Status 415) - Accept header matches what the Rest DSL produces. (Returns HTTP Status 406) - Missing required data (query parameters, HTTP headers, body). (Returns HTTP Status 400) Review comment: I wonder if we should set the reason of the issue in the response body, a dedicated header or both ? It could be in the scope of a separate ticket. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496941#comment-16496941 ] ASF GitHub Bot commented on CAMEL-11162: aldettinger opened a new pull request #2355: CAMEL-11162: Corrected doc and alleviated tests URL: https://github.com/apache/camel/pull/2355 @davsclaus This come as a late review as I was not able to grab free time earlier. Could you please comment ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496942#comment-16496942 ] ASF GitHub Bot commented on CAMEL-11162: GitHub user aldettinger opened a pull request: https://github.com/apache/camel/pull/2355 CAMEL-11162: Corrected doc and alleviated tests @davsclaus This come as a late review as I was not able to grab free time earlier. Could you please comment ? You can merge this pull request into a Git repository by running: $ git pull https://github.com/aldettinger/camel master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/camel/pull/2355.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2355 commit af4065152918b1011a7e4e1e836508eecf27d16e Author: aldettinger Date: 2018-05-31T17:48:40Z CAMEL-11162: Corrected doc and alleviated tests > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496240#comment-16496240 ] ASF GitHub Bot commented on CAMEL-11162: Github user davsclaus closed the pull request at: https://github.com/apache/camel/pull/2354 > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496239#comment-16496239 ] ASF GitHub Bot commented on CAMEL-11162: davsclaus closed pull request #2354: CAMEL-11162: rest-dsl add client request validation URL: https://github.com/apache/camel/pull/2354 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494914#comment-16494914 ] ASF GitHub Bot commented on CAMEL-11162: GitHub user davsclaus opened a pull request: https://github.com/apache/camel/pull/2354 CAMEL-11162: rest-dsl add client request validation So we check whether required data is included in the client http request when being processed by rest-dsl consumer. We now check for missing - query parameters - http headers - body The context-path is already handled in the uri path matcher, so it will fail already for invalid context-path. This can be turned on in the rest configuration. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/camel rest-val Alternatively you can review and apply these changes as the patch at: https://github.com/apache/camel/pull/2354.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2354 commit 8f088e7fa1fb11e48ed4150ad460bad1d4dd1020 Author: Claus Ibsen Date: 2018-05-29T14:38:28Z CAMEL-11162: rest-dsl now has validation check for consumer to check if the http client request has content-type/accept headers that is supported in rest-dsl consumes/produces settings. commit c9ff0ce02d9d7ed72e84b5fb58a19d7820822300 Author: Claus Ibsen Date: 2018-05-30T06:45:56Z CAMEL-11162: rest-dsl now has validation check for consumer to check if the http client request has content-type/accept headers that is supported in rest-dsl consumes/produces settings. commit 066566dea22ed6dcc2978ca33b0707a6d0035f7f Author: Claus Ibsen Date: 2018-05-30T09:03:49Z CAMEL-11162: rest-dsl now has validation check for consumer to check if the http client request has content-type/accept headers that is supported in rest-dsl consumes/produces settings. > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494913#comment-16494913 ] ASF GitHub Bot commented on CAMEL-11162: davsclaus opened a new pull request #2354: CAMEL-11162: rest-dsl add client request validation URL: https://github.com/apache/camel/pull/2354 So we check whether required data is included in the client http request when being processed by rest-dsl consumer. We now check for missing - query parameters - http headers - body The context-path is already handled in the uri path matcher, so it will fail already for invalid context-path. This can be turned on in the rest configuration. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (CAMEL-11162) camel-rest - Should we add content-type check for server side
[ https://issues.apache.org/jira/browse/CAMEL-11162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16493507#comment-16493507 ] Claus Ibsen commented on CAMEL-11162: - We would possible need to add a new option to turn this validation on|off. And then add some logic that checks for this: * server side consumes vs content-type provided by client * server side produces vs accept header from client (wonder if there is some special HTTP error code for this) * required parameters is included (body, path, query) * ??? > camel-rest - Should we add content-type check for server side > - > > Key: CAMEL-11162 > URL: https://issues.apache.org/jira/browse/CAMEL-11162 > Project: Camel > Issue Type: Improvement > Components: camel-core >Reporter: Claus Ibsen >Assignee: Claus Ibsen >Priority: Major > Fix For: 2.22.0 > > > For example setting up a rest-dsl which consumes application/json and then a > client calls it with text/plain or application/xml, should we then automatic > let rest consumer detect this and return a HTTP status 415 (unsuported media > type) > Not all the HTTP server components does this today, eg jetty etc. But when > using restlet which is more natual REST it would do so. > We could then add an option to turn this on|off. > The check is only if the media-type is within the list that may have been > specified on consumes in the rest-dsl. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
