You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "pvillard31 (via GitHub)" <gi...@apache.org> on 2023/04/28 17:11:51 UTC
[GitHub] [nifi] pvillard31 opened a new pull request, #7208: NIFI-11502 - Upgrade json-path to 2.8.0
pvillard31 opened a new pull request, #7208:
URL: https://github.com/apache/nifi/pull/7208
# Summary
[NIFI-11502](https://issues.apache.org/jira/browse/NIFI-11502) Upgrade json-path to 2.8.0
https://github.com/json-path/JsonPath/releases
Upgrade json-smart to fix https://www.cve.org/CVERecord?id=CVE-2023-1370
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [ ] Pull Request based on current revision of the `main` branch
- [ ] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 11
- [ ] JDK 17
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
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
[GitHub] [nifi] mr1716 commented on pull request #7208: NIFI-11502 - Upgrade json-path to 2.8.0
Posted by "mr1716 (via GitHub)" <gi...@apache.org>.
mr1716 commented on PR #7208:
URL: https://github.com/apache/nifi/pull/7208#issuecomment-1528188826
Might want to reference [NIFI-11374](https://issues.apache.org/jira/browse/NIFI-11374) as well
--
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
[GitHub] [nifi] mr1716 commented on pull request #7208: NIFI-11502 - Upgrade json-path to 2.8.0
Posted by "mr1716 (via GitHub)" <gi...@apache.org>.
mr1716 commented on PR #7208:
URL: https://github.com/apache/nifi/pull/7208#issuecomment-1528330866
> @pvillard31 Unfortunately it looks like JSON Path 2.8.0 introduced some changes the break existing unit tests. Further evaluation is necessary to determine whether the test expectations need to be adjusted, or whether the changes have broad impact in runtime deployments.
There are some modules in Nifi using json-path 2.6.9 and some using 2.7.0. If 2.8.0 can’t be used, the modules using 2.6.0 should be updated to 2.7.0
--
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
[GitHub] [nifi] asfgit closed pull request #7208: NIFI-11502 - Upgrade json-path to 2.8.0
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #7208: NIFI-11502 - Upgrade json-path to 2.8.0
URL: https://github.com/apache/nifi/pull/7208
--
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
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7208: NIFI-11502 - Upgrade json-path to 2.8.0
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #7208:
URL: https://github.com/apache/nifi/pull/7208#discussion_r1181102330
##########
nifi-commons/nifi-expression-language/src/main/java/org/apache/nifi/attribute/expression/language/evaluation/functions/JsonPathUpdateEvaluator.java:
##########
@@ -52,6 +54,13 @@ public QueryResult<String> evaluate(EvaluationContext context) {
String result;
try {
result = updateAttribute(documentContext, compiledJsonPath, value).jsonString();
+ } catch (PathNotFoundException pnf) {
+ // it is valid for a path not to be found, keys may not be there
+ // do not spam the error log for this, instead we can log debug if enabled
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("PathNotFoundException for JsonPath " + compiledJsonPath.getPath(), pnf);
+ }
Review Comment:
```suggestion
LOGGER.debug("JSON Path not found: {}", compiledJsonPath.getPath(), pnf);
```
##########
nifi-commons/nifi-expression-language/src/main/java/org/apache/nifi/attribute/expression/language/evaluation/functions/JsonPathDeleteEvaluator.java:
##########
@@ -42,6 +47,13 @@ public QueryResult<String> evaluate(EvaluationContext context) {
String result = null;
try {
result = documentContext.delete(compiledJsonPath).jsonString();
+ } catch (PathNotFoundException pnf) {
+ // it is valid for a path not to be found, keys may not be there
+ // do not spam the error log for this, instead we can log debug if enabled
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("PathNotFoundException for JsonPath " + compiledJsonPath.getPath(), pnf);
+ }
Review Comment:
The `isDebugEnabled()` check seems unnecessary since this is not an expensive operation, and it is already in the context of an exception. Placeholders should be used instead of string concatenation.
```suggestion
LOGGER.debug("JSON Path not found: {}", compiledJsonPath.getPath(), pnf);
```
--
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
[GitHub] [nifi] pvillard31 commented on pull request #7208: NIFI-11502 - Upgrade json-path to 2.8.0
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on PR #7208:
URL: https://github.com/apache/nifi/pull/7208#issuecomment-1528818480
After a quick investigation, with 2.6.0, if the JSON path is not resolving to anything we just return the initial JSON without any change and without logging any error. Starting with 2.7.0, if the JSON path does not resolve to an actual element, we throw an error. In order to not introduce any breaking change, I took the same approach as in JsonPathEvaluator with a debug logging and just return the JSON without any change.
--
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
[GitHub] [nifi] pvillard31 commented on pull request #7208: NIFI-11502 - Upgrade json-path to 2.8.0
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on PR #7208:
URL: https://github.com/apache/nifi/pull/7208#issuecomment-1528777524
Looking into it, the breaking change happens when going from `2.6.0` to `2.7.0`.
--
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