You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2023/08/12 13:15:12 UTC
[GitHub] [incubator-pekko] pjfanning opened a new pull request, #564: [DRAFT] jackson 2.15 support
pjfanning opened a new pull request, #564:
URL: https://github.com/apache/incubator-pekko/pull/564
see https://www.javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.15.2/com/fasterxml/jackson/core/StreamReadConstraints.html
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.15 support [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#discussion_r1367692755
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -37,6 +37,18 @@ pekko.serialization.jackson {
}
+#//#stream-read-constraints
+pekko.serialization.jackson {
+ read {
+ # see https://www.javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.15.2/com/fasterxml/jackson/core/StreamReadConstraints.html
+ # these defaults are the same as the defaults in `StreamReadConstraints`
+ max-nesting-depth = 1000
+ max-number-length = 1000
+ max-string-length = 20000000
Review Comment:
```
read.constraints {
max-nesting-depth = 1000
max-number-length = 1000
max-string-length = 20000000
}
```
Seems more clear
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.16 support [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on code in PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#discussion_r1435943862
##########
serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonObjectMapperProvider.scala:
##########
@@ -81,15 +83,31 @@ object JacksonObjectMapperProvider extends ExtensionId[JacksonObjectMapperProvid
config: Config,
baseJsonFactory: Option[JsonFactory]): JsonFactory = {
+ val streamReadConstraints = StreamReadConstraints.builder()
+ .maxNestingDepth(config.getInt("read.max-nesting-depth"))
+ .maxNumberLength(config.getInt("read.max-number-length"))
+ .maxStringLength(config.getInt("read.max-string-length"))
+ .maxNameLength(config.getInt("read.max-name-length"))
+ .maxDocumentLength(config.getLong("read.max-document-length"))
Review Comment:
Do these configurations have default values, or users must specify the configuration?
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.15 support [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#issuecomment-1805342596
jackson 2.16.0 should be out soon and it has some more constraints - I will repurpose this PR to further update to jackson 2.16.0 and support the extra constraints
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.16 support [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning merged PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.16 support [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#discussion_r1435997470
##########
serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonObjectMapperProvider.scala:
##########
@@ -81,15 +83,31 @@ object JacksonObjectMapperProvider extends ExtensionId[JacksonObjectMapperProvid
config: Config,
baseJsonFactory: Option[JsonFactory]): JsonFactory = {
+ val streamReadConstraints = StreamReadConstraints.builder()
+ .maxNestingDepth(config.getInt("read.max-nesting-depth"))
+ .maxNumberLength(config.getInt("read.max-number-length"))
+ .maxStringLength(config.getInt("read.max-string-length"))
+ .maxNameLength(config.getInt("read.max-name-length"))
+ .maxDocumentLength(config.getLong("read.max-document-length"))
Review Comment:
This is how Akka/Pekko has always worked.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.15 support [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#discussion_r1368625226
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -37,6 +37,18 @@ pekko.serialization.jackson {
}
+#//#stream-read-constraints
+pekko.serialization.jackson {
+ read {
+ # see https://www.javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.15.2/com/fasterxml/jackson/core/StreamReadConstraints.html
+ # these defaults are the same as the defaults in `StreamReadConstraints`
+ max-nesting-depth = 1000
+ max-number-length = 1000
+ max-string-length = 20000000
Review Comment:
I honestly don't think adding the word `constraints` to the config names here makes them any clearer. Config names need to balance descriptiveness with conciseness.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.16 support [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#issuecomment-1879629323
Okay ill look at it in more detail tomorrow
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.16 support [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#discussion_r1435997215
##########
serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonObjectMapperProvider.scala:
##########
@@ -81,15 +83,31 @@ object JacksonObjectMapperProvider extends ExtensionId[JacksonObjectMapperProvid
config: Config,
baseJsonFactory: Option[JsonFactory]): JsonFactory = {
+ val streamReadConstraints = StreamReadConstraints.builder()
+ .maxNestingDepth(config.getInt("read.max-nesting-depth"))
+ .maxNumberLength(config.getInt("read.max-number-length"))
+ .maxStringLength(config.getInt("read.max-string-length"))
+ .maxNameLength(config.getInt("read.max-name-length"))
+ .maxDocumentLength(config.getLong("read.max-document-length"))
Review Comment:
We ship reference conf with the defaults. Users can override the values by setting them in their own application.conf or application.json.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.16 support [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#issuecomment-1879627875
@mdedetrich if we are getting close to creating an RC for the M1 of pekko 1.1.0, I would like to get this merged.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.15 support [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#discussion_r1367692755
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -37,6 +37,18 @@ pekko.serialization.jackson {
}
+#//#stream-read-constraints
+pekko.serialization.jackson {
+ read {
+ # see https://www.javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.15.2/com/fasterxml/jackson/core/StreamReadConstraints.html
+ # these defaults are the same as the defaults in `StreamReadConstraints`
+ max-nesting-depth = 1000
+ max-number-length = 1000
+ max-string-length = 20000000
Review Comment:
```
constraints {
max-nesting-depth = 1000
max-number-length = 1000
max-string-length = 20000000
}
```
Seems more clear
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.16 support [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#issuecomment-1872259129
@He-Pin @mdedetrich is this something we can merge into main branch? Jackson 2.16 has quite a bit of security hardening.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] jackson 2.16 support [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #564:
URL: https://github.com/apache/incubator-pekko/pull/564#discussion_r1443791110
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -37,6 +37,26 @@ pekko.serialization.jackson {
}
+#//#stream-read-constraints
+pekko.serialization.jackson {
+ read {
+ # see https://www.javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.0/com/fasterxml/jackson/core/StreamReadConstraints.html
+ # these defaults are the same as the defaults in `StreamReadConstraints`
+ max-nesting-depth = 1000
+ max-number-length = 1000
+ max-string-length = 20000000
+ max-name-length = 50000
+ # max-document-length of -1 means unlimited
+ max-document-length = -1
+ }
Review Comment:
Add a blank line here
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org