You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "lordgamez (via GitHub)" <gi...@apache.org> on 2024/03/19 12:29:57 UTC
[PR] MINIFICPP-2224 Drop support for disabling peer verification in InvokeHTTP [nifi-minifi-cpp]
lordgamez opened a new pull request, #1745:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1745
https://issues.apache.org/jira/browse/MINIFICPP-2224
--------------------
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2224 Drop support for disabling peer verification in InvokeHTTP [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1745:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1745#discussion_r1532300544
##########
PROCESSORS.md:
##########
@@ -1358,7 +1358,7 @@ In the list below, the names of required properties appear in bold. Any other pr
| send-message-body | true | true<br/>false | DEPRECATED. Only kept for backwards compatibility, no functionality is included. |
| Send Message Body | true | true<br/>false | If true, sends the HTTP message body on POST/PUT/PATCH requests (default). If false, suppresses the message body and content-type header for these requests. |
| Use Chunked Encoding | false | true<br/>false | When POST'ing, PUT'ing or PATCH'ing content set this property to true in order to not pass the 'Content-length' header and instead send 'Transfer-Encoding' with a value of 'chunked'. This will enable the data transfer mechanism which was introduced in HTTP 1.1 to pass data of unknown lengths in chunks. |
-| Disable Peer Verification | false | true<br/>false | Disables peer verification for the SSL session |
+| Disable Peer Verification | false | true<br/>false | DEPRECATED. Only kept for backwards compatibility, no functionality is included. |
Review Comment:
Makes sense, updated in 55a700da1079435db2a352da9e672c6fa5ce2501
##########
extensions/http-curl/processors/InvokeHTTP.h:
##########
@@ -150,7 +150,7 @@ class InvokeHTTP : public core::Processor {
.withDefaultValue("false")
.build();
EXTENSIONAPI static constexpr auto DisablePeerVerification = core::PropertyDefinitionBuilder<>::createProperty("Disable Peer Verification")
- .withDescription("Disables peer verification for the SSL session")
+ .withDescription("DEPRECATED. Only kept for backwards compatibility, no functionality is included.")
Review Comment:
Updated in 55a700da1079435db2a352da9e672c6fa5ce2501
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2224 Drop support for disabling peer verification in InvokeHTTP [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1745:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1745#discussion_r1530468857
##########
PROCESSORS.md:
##########
@@ -1358,7 +1358,7 @@ In the list below, the names of required properties appear in bold. Any other pr
| send-message-body | true | true<br/>false | DEPRECATED. Only kept for backwards compatibility, no functionality is included. |
| Send Message Body | true | true<br/>false | If true, sends the HTTP message body on POST/PUT/PATCH requests (default). If false, suppresses the message body and content-type header for these requests. |
| Use Chunked Encoding | false | true<br/>false | When POST'ing, PUT'ing or PATCH'ing content set this property to true in order to not pass the 'Content-length' header and instead send 'Transfer-Encoding' with a value of 'chunked'. This will enable the data transfer mechanism which was introduced in HTTP 1.1 to pass data of unknown lengths in chunks. |
-| Disable Peer Verification | false | true<br/>false | Disables peer verification for the SSL session |
+| Disable Peer Verification | false | true<br/>false | DEPRECATED. Only kept for backwards compatibility, no functionality is included. |
Review Comment:
The word "deprecated" already communicates that this is only kept for backwards compatibility. I suggest changing the sentence to describe how the value is used, and what's the actual behavior. Users reading this description are most likely interested in these details.
```suggestion
| Disable Peer Verification | false | true<br/>false | DEPRECATED. The value is ignored, peer and host verification are always performed when using SSL/TLS. |
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2224 Drop support for disabling peer verification in InvokeHTTP [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1745:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1745#discussion_r1530468857
##########
PROCESSORS.md:
##########
@@ -1358,7 +1358,7 @@ In the list below, the names of required properties appear in bold. Any other pr
| send-message-body | true | true<br/>false | DEPRECATED. Only kept for backwards compatibility, no functionality is included. |
| Send Message Body | true | true<br/>false | If true, sends the HTTP message body on POST/PUT/PATCH requests (default). If false, suppresses the message body and content-type header for these requests. |
| Use Chunked Encoding | false | true<br/>false | When POST'ing, PUT'ing or PATCH'ing content set this property to true in order to not pass the 'Content-length' header and instead send 'Transfer-Encoding' with a value of 'chunked'. This will enable the data transfer mechanism which was introduced in HTTP 1.1 to pass data of unknown lengths in chunks. |
-| Disable Peer Verification | false | true<br/>false | Disables peer verification for the SSL session |
+| Disable Peer Verification | false | true<br/>false | DEPRECATED. Only kept for backwards compatibility, no functionality is included. |
Review Comment:
Deprecated already communicates that this is only kept for backwards compatibility. I suggest changing the sentence to describe how the value is used, and what's the actual behavior. Users reading this description are most likely interested in these details.
```suggestion
| Disable Peer Verification | false | true<br/>false | DEPRECATED. The value is ignored, peer and host verification are always performed when using SSL/TLS. |
```
##########
extensions/http-curl/processors/InvokeHTTP.h:
##########
@@ -150,7 +150,7 @@ class InvokeHTTP : public core::Processor {
.withDefaultValue("false")
.build();
EXTENSIONAPI static constexpr auto DisablePeerVerification = core::PropertyDefinitionBuilder<>::createProperty("Disable Peer Verification")
- .withDescription("Disables peer verification for the SSL session")
+ .withDescription("DEPRECATED. Only kept for backwards compatibility, no functionality is included.")
Review Comment:
```suggestion
.withDescription("DEPRECATED. The value is ignored, peer and host verification are always performed when using SSL/TLS.")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2224 Drop support for disabling peer verification in InvokeHTTP [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm closed pull request #1745: MINIFICPP-2224 Drop support for disabling peer verification in InvokeHTTP
URL: https://github.com/apache/nifi-minifi-cpp/pull/1745
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org