You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by "cwperks (via GitHub)" <gi...@apache.org> on 2024/05/15 01:01:34 UTC
[PR] Remove h from ESCAPED_CHARS in JsonMapObjectReaderWriter [cxf]
cwperks opened a new pull request, #1872:
URL: https://github.com/apache/cxf/pull/1872
The JsonMapObjectReaderWriter class maintains a list of [ESCAPED_CHARS](https://github.com/apache/cxf/blob/main/rt/rs/extensions/json-basic/src/main/java/org/apache/cxf/jaxrs/json/basic/JsonMapObjectReaderWriter.java#L45-L56) which includes special characters that need to be escaped line the newline (`\n`) and tab (`\t`) characters. This list also includes `\h`, but I can't find any links to official documentation about this character needing to be escaped.
According to this [SO post](https://stackoverflow.com/a/27516892) which details escaped characters in JSON, it also does not include `\h` in this list.
I'm opening a PR to discuss removing this from the list.
Issue in OpenSearch where this issue is discussed: https://github.com/opensearch-project/security/issues/2531#issuecomment-2111309193
Simple test case which demonstrates the difference:
```
package org.opensearch.security.auth;
import org.apache.cxf.rs.security.jose.jwk.JsonWebKey;
import org.apache.cxf.rs.security.jose.jwk.KeyType;
import org.apache.cxf.rs.security.jose.jwk.PublicKeyUse;
import org.apache.cxf.rs.security.jose.jws.JwsUtils;
import org.apache.cxf.rs.security.jose.jwt.JoseJwtProducer;
import org.apache.cxf.rs.security.jose.jwt.JwtClaims;
import org.apache.cxf.rs.security.jose.jwt.JwtToken;
import org.apache.cxf.rs.security.jose.jwt.JwtUtils;
import org.junit.Test;
public class CXFTests {
@Test
public void testBase64URLUtility() {
JoseJwtProducer jwtProducer = new JoseJwtProducer();
JsonWebKey jwk = new JsonWebKey();
jwk.setKeyType(KeyType.OCTET);
jwk.setAlgorithm("HS512");
jwk.setPublicKeyUse(PublicKeyUse.SIGN);
jwk.setProperty("k", "TXkgU0FNTCBTZWNyZXQgZXhjaGFuZ2Uga2V5LiBBIHNlY3JldCB0aGF0IG5vIG9uZSBvciBib3RuZXQgaW4gdGhlIHdvcmxkIHdpbGwgZXZlciBiZSBhYmxlIHRvIGNyYWNrLg==");
jwtProducer.setSignatureProvider(JwsUtils.getSignatureProvider(jwk));
JwtClaims jwtClaims1 = new JwtClaims();
JwtToken jwt1 = new JwtToken(jwtClaims1);
jwtClaims1.setSubject("user1\\nexample");
JwtClaims jwtClaims2 = new JwtClaims();
JwtToken jwt2 = new JwtToken(jwtClaims2);
jwtClaims2.setSubject("user2\\hexample");
String encodedJwt1 = jwtProducer.processJwt(jwt1);
String encodedJwt2 = jwtProducer.processJwt(jwt2);
System.out.println("encodedJwt1: " + encodedJwt1);
System.out.println("encodedJwt2: " + encodedJwt2);
System.out.println("jwt1.toJson: " + JwtUtils.claimsToJson(jwt1.getClaims(), null));
System.out.println("jwt2.toJson: " + JwtUtils.claimsToJson(jwt2.getClaims(), null));
}
}
```
Output:
```
encodedJwt1: eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJ1c2VyMVxcbmV4YW1wbGUifQ.iFP82Rq_EsEDkL4X7gaa21q0k4Yt81ybMWnqM2QgS55R1xKE8NARWZEQrjvA5lZwT4368-61BUeix7ufmdOz0A
encodedJwt2: eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJ1c2VyMlxoZXhhbXBsZSJ9.O-glz6XD7guP4DemL6nyAnEVgGb4XcyLprtcHnpVGoFCe37eEW7UFi8MTmJ8D3JvJoLso1OfKiLkYTHjRecUcw
jwt1.toJson: {"sub":"user1\\nexample"}
jwt2.toJson: {"sub":"user2\hexample"}
```
jwt2 has an invalid payload.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove h from ESCAPED_CHARS in JsonMapObjectReaderWriter [cxf]
Posted by "cwperks (via GitHub)" <gi...@apache.org>.
cwperks commented on code in PR #1872:
URL: https://github.com/apache/cxf/pull/1872#discussion_r1600826579
##########
rt/rs/extensions/json-basic/src/main/java/org/apache/cxf/jaxrs/json/basic/JsonMapObjectReaderWriter.java:
##########
@@ -49,7 +49,6 @@ public class JsonMapObjectReaderWriter {
chars.add('/');
chars.add('b');
chars.add('f');
Review Comment:
Changed this to `n`. Good catch!
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove h from ESCAPED_CHARS in JsonMapObjectReaderWriter [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #1872:
URL: https://github.com/apache/cxf/pull/1872#discussion_r1600827091
##########
rt/rs/extensions/json-basic/src/main/java/org/apache/cxf/jaxrs/json/basic/JsonMapObjectReaderWriter.java:
##########
@@ -49,7 +49,6 @@ public class JsonMapObjectReaderWriter {
chars.add('/');
chars.add('b');
chars.add('f');
Review Comment:
@coheigea could you please take a look? very minor change, thank you
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove h from ESCAPED_CHARS in JsonMapObjectReaderWriter [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta merged PR #1872:
URL: https://github.com/apache/cxf/pull/1872
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove h from ESCAPED_CHARS in JsonMapObjectReaderWriter [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #1872:
URL: https://github.com/apache/cxf/pull/1872#discussion_r1600825436
##########
rt/rs/extensions/json-basic/src/main/java/org/apache/cxf/jaxrs/json/basic/JsonMapObjectReaderWriter.java:
##########
@@ -49,7 +49,6 @@ public class JsonMapObjectReaderWriter {
chars.add('/');
chars.add('b');
chars.add('f');
Review Comment:
```
chars.add('n');
```
I believe it was a typo (https://datatracker.ietf.org/doc/html/rfc8259#section-7)L
```
string = quotation-mark *char quotation-mark
char = unescaped /
escape (
%x22 / ; " quotation mark U+0022
%x5C / ; \ reverse solidus U+005C
%x2F / ; / solidus U+002F
%x62 / ; b backspace U+0008
%x66 / ; f form feed U+000C
%x6E / ; n line feed U+000A
%x72 / ; r carriage return U+000D
%x74 / ; t tab U+0009
%x75 4HEXDIG ) ; uXXXX U+XXXX
escape = %x5C ; \
quotation-mark = %x22 ; "
unescaped = %x20-21 / %x23-5B / %x5D-10FFFF
```
##########
rt/rs/extensions/json-basic/src/main/java/org/apache/cxf/jaxrs/json/basic/JsonMapObjectReaderWriter.java:
##########
@@ -49,7 +49,6 @@ public class JsonMapObjectReaderWriter {
chars.add('/');
chars.add('b');
chars.add('f');
Review Comment:
```
chars.add('n');
```
I believe it was a typo (https://datatracker.ietf.org/doc/html/rfc8259#section-7)L
```
string = quotation-mark *char quotation-mark
char = unescaped /
escape (
%x22 / ; " quotation mark U+0022
%x5C / ; \ reverse solidus U+005C
%x2F / ; / solidus U+002F
%x62 / ; b backspace U+0008
%x66 / ; f form feed U+000C
%x6E / ; n line feed U+000A
%x72 / ; r carriage return U+000D
%x74 / ; t tab U+0009
%x75 4HEXDIG ) ; uXXXX U+XXXX
escape = %x5C ; \
quotation-mark = %x22 ; "
unescaped = %x20-21 / %x23-5B / %x5D-10FFFF
```
Thanks a lot @cwperks !
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove h from ESCAPED_CHARS in JsonMapObjectReaderWriter [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #1872:
URL: https://github.com/apache/cxf/pull/1872#discussion_r1600827091
##########
rt/rs/extensions/json-basic/src/main/java/org/apache/cxf/jaxrs/json/basic/JsonMapObjectReaderWriter.java:
##########
@@ -49,7 +49,6 @@ public class JsonMapObjectReaderWriter {
chars.add('/');
chars.add('b');
chars.add('f');
Review Comment:
@coheigea could you please take a look? very minor change, thank you (I think a typo sneaked in during https://issues.apache.org/jira/browse/CXF-8555)
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove h from ESCAPED_CHARS in JsonMapObjectReaderWriter [cxf]
Posted by "cwperks (via GitHub)" <gi...@apache.org>.
cwperks commented on PR #1872:
URL: https://github.com/apache/cxf/pull/1872#issuecomment-2123060947
@coheigea Could you review this 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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove h from ESCAPED_CHARS in JsonMapObjectReaderWriter [cxf]
Posted by "cwperks (via GitHub)" <gi...@apache.org>.
cwperks commented on PR #1872:
URL: https://github.com/apache/cxf/pull/1872#issuecomment-2111401984
@reta Could you take a look? This is related to an issue in the OpenSearch security plugin.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org