You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by pvillard31 <gi...@git.apache.org> on 2016/03/12 17:25:42 UTC
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
GitHub user pvillard31 opened a pull request:
https://github.com/apache/nifi/pull/272
NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pvillard31/nifi NIFI-1620
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/272.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 #272
----
commit 490278016a2df854c2cece9047a0e1e3e10f213f
Author: Pierre Villard <pi...@gmail.com>
Date: 2016-03-12T16:13:04Z
NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in
InvokeHTTP processor
Posted by Adam Taft <ad...@adamtaft.com>.
I added a comment to the JIRA ticket associated with this pull request. I
think there should be discussion / buy-in from others on the aestetics of
introducing a new processor property for this edge case. Instead, I think
the goals of this request could be fulfilled without strictly introducing a
new property, which I think would be a likely improved approach.
https://issues.apache.org/jira/browse/NIFI-1620
Maybe we should postpone this ticket resolution and not merge in 0.7.x
until more discussion has occurred? I wouldn't want to merge this change
without at least a few nods agreeing to the proposed property.
On Wed, Jun 15, 2016 at 5:05 PM, JPercivall <gi...@git.apache.org> wrote:
> Github user JPercivall commented on the issue:
>
> https://github.com/apache/nifi/pull/272
>
> @taftster I'll let you finish it up tonight if you have time since
> you've already had eyes on it. If you're not able to, I'll take a look
> tomorrow.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
[GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/272
@taftster I'll let you finish it up tonight if you have time since you've already had eyes on it. If you're not able to, I'll take a look tomorrow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-199144952
I updated the PR with a true/false "send body" property, should be in line with last comments. Let me know if not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-196284703
Thank you @pvillard31 . That does make sense and in the case that there is no request body being sent over I am supportive of the notion of not sending the content type header. This also seems in line with Adam's rfc reference. You want to tweak this PR then to retain existing content header logic but rather have a 'send body' property or something which when false will not send the body and will not set that header?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on the issue:
https://github.com/apache/nifi/pull/272
I'm happy with this PR. Thanks @pvillard31 for all the effort and seeing this through many changes and discussions.
@JPercivall can you still merge this? I'm in between environments right now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-196309847
@joewitt yes I can have a look this afternoon. I'll try to propose something before the end of the day.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-196200486
Definitely agree with you that this API should not act this way.
Anyway, just to clarify, this is only in the case there is no body/content sent with the request.
That's why, in a message on the mailing list, I suggested to add a "body" property. If this property is set (possibly set to empty), the possible (incoming relationship is not mandatory with this processor) incoming flow file is ignored to set content and the request body is constructed with the property. This way we can validate the empty content type if and only if body is set to empty.
Let me know if that makes sense.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP pro...
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on a diff in the pull request:
https://github.com/apache/nifi/pull/272#discussion_r67283832
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java ---
@@ -215,14 +215,23 @@
.build();
public static final PropertyDescriptor PROP_CONTENT_TYPE = new PropertyDescriptor.Builder()
- .name("Content-Type")
- .description("The Content-Type to specify for when content is being transmitted through a PUT or POST. "
- + "In the case of an empty value after evaluating an expression language expression, Content-Type defaults to " + DEFAULT_CONTENT_TYPE)
- .required(true)
- .expressionLanguageSupported(true)
- .defaultValue("${" + CoreAttributes.MIME_TYPE.key() + "}")
- .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
- .build();
+ .name("Content-Type")
+ .description("The Content-Type to specify for when content is being transmitted through a PUT or POST. "
+ + "In the case of an empty value after evaluating an expression language expression, Content-Type defaults to " + DEFAULT_CONTENT_TYPE)
+ .required(true)
+ .expressionLanguageSupported(true)
+ .defaultValue("${" + CoreAttributes.MIME_TYPE.key() + "}")
+ .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING))
+ .build();
+
--- End diff --
The changes on lines 218-226 are just whitespace changes. Can this be reverted back to the original?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-195793923
Due to JIRA unavailability, I think the PR has not been linked to issue in JIRA. Just adding a comment to mention the PR in JIRA.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on a diff in the pull request:
https://github.com/apache/nifi/pull/272#discussion_r56928549
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java ---
@@ -444,14 +455,27 @@ public void onPropertyModified(final PropertyDescriptor descriptor, final String
@Override
protected Collection<ValidationResult> customValidate(final ValidationContext validationContext) {
- final List<ValidationResult> results = new ArrayList<>(1);
+ final List<ValidationResult> results = new ArrayList<ValidationResult>(3);
+
+ // proxy host and port validation
final boolean proxyHostSet = validationContext.getProperty(PROP_PROXY_HOST).isSet();
final boolean proxyPortSet = validationContext.getProperty(PROP_PROXY_PORT).isSet();
-
if ((proxyHostSet && !proxyPortSet) || (!proxyHostSet && proxyPortSet)) {
results.add(new ValidationResult.Builder().subject("Proxy Host and Port").valid(false).explanation("If Proxy Host or Proxy Port is set, both must be set").build());
}
+ // body and content-type validation
+ final boolean contentTypeSet = validationContext.getProperty(PROP_CONTENT_TYPE).isSet();
+ final boolean sendBody = validationContext.getProperty(PROP_SEND_BODY).asBoolean();
+ final String contentType = validationContext.getProperty(PROP_CONTENT_TYPE).getValue();
+ if(contentTypeSet && contentType.isEmpty() && sendBody) {
+ results.add(new ValidationResult.Builder().subject("Content-Type and Send body")
+ .valid(false).explanation("If Content-Type is set to empty, Send body property must be set to false").build());
+ }
+ if(!sendBody && !contentType.isEmpty()) {
+ results.add(new ValidationResult.Builder().subject("Content-Type and Send body").valid(false).explanation("If body is not sent, Content-Type must be set to empty").build());
+ }
+
return results;
--- End diff --
This custom validation rule can be removed if the content-type property is left alone.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-198776846
Should be OK now :
- Added body property
- Reverted default Content-Type and original unit tests
- Authorized no incoming connection with POST/PUT
- Added check : if content-type empty, body must be empty
- Added check : if body empty, content-type must be empty
- Added unit tests
- Tested against the API at the origin of the JIRA
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on a diff in the pull request:
https://github.com/apache/nifi/pull/272#discussion_r56928547
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java ---
@@ -215,14 +215,24 @@
.build();
public static final PropertyDescriptor PROP_CONTENT_TYPE = new PropertyDescriptor.Builder()
- .name("Content-Type")
- .description("The Content-Type to specify for when content is being transmitted through a PUT or POST. "
- + "In the case of an empty value after evaluating an expression language expression, Content-Type defaults to " + DEFAULT_CONTENT_TYPE)
- .required(true)
- .expressionLanguageSupported(true)
- .defaultValue("${" + CoreAttributes.MIME_TYPE.key() + "}")
- .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
- .build();
+ .name("Content-Type")
+ .description("The Content-Type to specify for when content is being transmitted through a PUT or POST. "
+ + "In the case of an empty value after evaluating an expression language expression, Content-Type defaults to " + DEFAULT_CONTENT_TYPE + "."
+ + "If and only if body is not sent, Content-Type must be set to empty")
+ .required(false)
+ .expressionLanguageSupported(true)
+ .defaultValue("${" + CoreAttributes.MIME_TYPE.key() + "}")
+ .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING))
+ .build();
+
+ public static final PropertyDescriptor PROP_SEND_BODY = new PropertyDescriptor.Builder()
+ .name("Send body")
+ .description("True if the body content should be sent, false otherwise")
+ .defaultValue("true")
--- End diff --
We should refer to this as the HTTP spec refers to it. It's called the "message-body" in the spec, so I think we should use that here as well. i.e.
```
.name("send-message-body")
.displayName("Send Message Body")
.description("If true, sends the HTTP message body on POST/PUT requests (default). If false, suppresses the message body and content-type header for these requests.")
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/272
@taftster will you have time to finish up this review for 0.7.0 or do you want me to finish it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the issue:
https://github.com/apache/nifi/pull/272
It should be OK, let me know if not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-196786056
Not sure to be able to propose something today, I don't have as much free time as expected. I believe this should be moved to 0.7.0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/272
Thanks for the assist @apiri
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-196089886
i was reviewing this earlier today and frankly had a similar concern to this as Adam. I didn't reply because I hadn't really figured out what to think. First, I agree that a service which rejects that header is arguably broken. Second, as the patch is right now I am curious how it works when the value is empty string because there is a static call to MediaType.... which seems like it would have trouble (still need to verify the logic there though).
However, having said this Pierre can you clarify if the intent is only for the case where there is no entity body or is it also for when there is an entity body in the request? If the idea is that this is only necessary when there is no entity body we should tighten the code for that case and if it is for either scenario then I think i'm of similar mind to Adam here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/272
@taftster thanks for taking another look, yup I can merge this when I get a chance.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-199086549
@pvillard31 If you stay focused on the goal of NIFI-1620 here and avoid the body manipulation this seems like a good step. Clearly you found a case worthy of support (null body means unset or empty strong content type). But, i do share Adam's view that it is best to avoid the content manipulation here. Do you agree or do you feel like for the use case(s) you envisioned this would be problematic?
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on the issue:
https://github.com/apache/nifi/pull/272
I can try to look at it tonight. But no problems if you want to step in.
> On Jun 15, 2016, at 10:51 AM, Joe Percivall <no...@github.com> wrote:
>
> @taftster will you have time to finish up this review for 0.7.0 or do you want me to finish it?
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on a diff in the pull request:
https://github.com/apache/nifi/pull/272#discussion_r56928543
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java ---
@@ -215,14 +215,24 @@
.build();
public static final PropertyDescriptor PROP_CONTENT_TYPE = new PropertyDescriptor.Builder()
- .name("Content-Type")
- .description("The Content-Type to specify for when content is being transmitted through a PUT or POST. "
- + "In the case of an empty value after evaluating an expression language expression, Content-Type defaults to " + DEFAULT_CONTENT_TYPE)
- .required(true)
- .expressionLanguageSupported(true)
- .defaultValue("${" + CoreAttributes.MIME_TYPE.key() + "}")
- .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
- .build();
+ .name("Content-Type")
+ .description("The Content-Type to specify for when content is being transmitted through a PUT or POST. "
+ + "In the case of an empty value after evaluating an expression language expression, Content-Type defaults to " + DEFAULT_CONTENT_TYPE + "."
+ + "If and only if body is not sent, Content-Type must be set to empty")
+ .required(false)
+ .expressionLanguageSupported(true)
+ .defaultValue("${" + CoreAttributes.MIME_TYPE.key() + "}")
+ .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING))
+ .build();
+
--- End diff --
I'm not sure if we even need this change. In the case that the PROP_SEND_BODY is false, we should always suppress setting the Content-Type header. Therefore, I would leave this as-is a required field, with documentation in the PROP_SEND_FIELD property describing the behavior for empty message bodies.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-199086081
This pull request is not strictly dealing with NIFI-1620. The edge case of suppressing the Content-Type header for an empty message-body makes sense to fix and address. NIFI-1620 is dealing with this edge case.
However, this PR is adding the ability to specify an HTTP request message-body directly as a property of InvokeHTTP. In my opinion, this is not a desired feature, and I am -1 on this. A static message-body can always be provided in a dataflow using a tool like ReplaceText; keeping the semantics that InvokeHTTP always reads from flowfile payloads has a good consistency.
Recommend that this PR focus solely on the problem dealing with Content-Type of an empty payload for a POST/PUT operation. Another JIRA can be opened to discuss the merits of how to provide static content to an HTTP transaction.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP pro...
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on a diff in the pull request:
https://github.com/apache/nifi/pull/272#discussion_r67284145
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java ---
@@ -761,24 +771,38 @@ private Request configureRequest(final ProcessContext context, final ProcessSess
}
private RequestBody getRequestBodyToSend(final ProcessSession session, final ProcessContext context, final FlowFile requestFlowFile) {
- return new RequestBody() {
- @Override
- public MediaType contentType() {
- String contentType = context.getProperty(PROP_CONTENT_TYPE).evaluateAttributeExpressions(requestFlowFile).getValue();
- contentType = StringUtils.isBlank(contentType) ? DEFAULT_CONTENT_TYPE : contentType;
- return MediaType.parse(contentType);
- }
+ if(context.getProperty(PROP_SEND_BODY).asBoolean()) {
+ return new RequestBody() {
+ @Override
+ public MediaType contentType() {
+ String contentType = context.getProperty(PROP_CONTENT_TYPE).evaluateAttributeExpressions(requestFlowFile).getValue();
+ contentType = StringUtils.isBlank(contentType) ? DEFAULT_CONTENT_TYPE : contentType;
+ return MediaType.parse(contentType);
+ }
- @Override
- public void writeTo(BufferedSink sink) throws IOException {
- session.exportTo(requestFlowFile, sink.outputStream());
- }
+ @Override
+ public void writeTo(BufferedSink sink) throws IOException {
+ session.exportTo(requestFlowFile, sink.outputStream());
+ }
- @Override
- public long contentLength(){
- return useChunked ? -1 : requestFlowFile.getSize();
- }
- };
+ @Override
+ public long contentLength(){
+ return useChunked ? -1 : requestFlowFile.getSize();
+ }
+ };
+ } else {
+ return new RequestBody() {
+ @Override
+ public void writeTo(BufferedSink sink) throws IOException {
+ sink.writeUtf8("");
+ }
+
+ @Override
+ public MediaType contentType() {
+ return MediaType.parse("");
+ }
+ };
--- End diff --
For this 'else' clause, it would probably be better to use a RequstBody static factory. Something like:
`return RequestBody.create(null, new byte[0]);`
I'm not sure the proper way to specific an empty MediaType, though. I think reading the MediaType javadoc, my hunch is that MediaType.parse("") should return null (since empty string is an invalid type).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP processor
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi/pull/272
@taftster @JPercivall I'll aid in the assist and wrap this up as I have been scanning through PRs and can merge in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #272: NIFI-1620 Allow empty Content-Type in InvokeHTTP pro...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/272
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by taftster <gi...@git.apache.org>.
Github user taftster commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-196079414
I'm not entirely sure if this is a good idea. Any web service which _disallows_ a standard HTTP header is arguably broken. Quoting RFC 2616:
> Any HTTP/1.1 message containing an entity-body SHOULD include a
> Content-Type header field defining the media type of that body. If
> and only if the media type is not given by a Content-Type field, the
> recipient MAY attempt to guess the media type via inspection of its
> content and/or the name extension(s) of the URI used to identify the
> resource. If the media type remains unknown, the recipient SHOULD
> treat it as type "application/octet-stream".
It seems hard to believe given the above that a web service would reject a response with Content-Type. The current behavior of InvokeHTTP is possibly the most consistent with the HTTP specification.
A custom processor designed to specifically interact with the remote service in question should be considered as an alternative to modifying InvokeHTTP.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1620 Allow empty Content-Type in InvokeHTT...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the pull request:
https://github.com/apache/nifi/pull/272#issuecomment-199714780
@taftster Thanks Adam. I updated the PR regarding your comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---