You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "heliang666s (via GitHub)" <gi...@apache.org> on 2024/08/18 11:54:39 UTC
[PR] Fix fury bug [dubbo-spi-extensions]
heliang666s opened a new pull request, #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476
## What is the purpose of the change
Fix fury bug
https://github.com/apache/dubbo/issues/14536
## Brief changelog
Fix FuryInputStream.java.By the way,add more tests.
## Verifying this change
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Make sure there is a [GITHUB_issue](https://github.com/apache/dubbo/issues) field for the change (usually before
you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address
just this issue, without pulling in other changes - one PR resolves one issue.
- [ ] Format the pull request title like `[Dubbo-XXX] Fix UnknownException when host config not exist #XXX`. Each commit
in the pull request should have a meaningful subject line and body.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency
exist. If the new feature or significant change is committed, please remember to add sample
in [dubbo samples](https://github.com/apache/dubbo-samples) project.
- [ ] Run `mvn clean install -DskipTests=false` & `mvn clean test-compile failsafe:integration-test` to make sure
unit-test and integration-test pass.
- [ ] If this contribution is large, please follow
the [Software Donation Guide](https://github.com/apache/dubbo/wiki/Software-donation-guide).
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476#discussion_r1724672471
##########
dubbo-serialization-extensions/dubbo-serialization-fury/src/main/java/org/apache/dubbo/common/serialize/fury/dubbo/FuryObjectInput.java:
##########
@@ -40,7 +40,7 @@ public FuryObjectInput(Fury fury, MemoryBuffer buffer, InputStream input) {
@Override
public Object readObject() {
- return fury.deserialize(new FuryInputStream(input, buffer.size()));
+ return fury.deserializeJavaObjectAndClass(new FuryInputStream(input, buffer.size()));
Review Comment:
If the passed `InputStream` is used by dubbo after fury consumed more data, the usage in dubbo may fail. If we created `FuryInputStream`, we should use `FuryInputStream` in all places when using original `InputStream` just like we use `BufferedInputStream`
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "yunshen-Lee (via GitHub)" <gi...@apache.org>.
yunshen-Lee commented on PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476#issuecomment-2295609059
After testing, there are still issues with deserialization.
Caused by: java.lang.ArrayIndexOutOfBoundsException: 49
at org.apache.fury.resolver.MetaStringBytes.decode(MetaStringBytes.java:69) ~[fury-core-0.7.0.jar!/:0.7.0]
at org.apache.fury.resolver.ClassResolver.populateBytesToClassInfo(ClassResolver.java:1708) ~[fury-core-0.7.0.jar!/:0.7.0]
at org.apache.fury.resolver.ClassResolver.loadBytesToClassInfo(ClassResolver.java:1699) ~[fury-core-0.7.0.jar!/:0.7.0]
at org.apache.fury.resolver.ClassResolver.readClassInfoFromBytes(ClassResolver.java:1686) ~[fury-core-0.7.0.jar!/:0.7.0]
at org.apache.fury.resolver.ClassResolver.readClassInfo(ClassResolver.java:1608) ~[fury-core-0.7.0.jar!/:0.7.0]
at org.apache.fury.Fury.readRef(Fury.java:857) ~[fury-core-0.7.0.jar!/:0.7.0]
at org.apache.fury.Fury.deserializeJavaObjectAndClass(Fury.java:1225) ~[fury-core-0.7.0.jar!/:0.7.0]
at org.apache.fury.Fury.deserializeJavaObjectAndClass(Fury.java:1245) ~[fury-core-0.7.0.jar!/:0.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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "chickenlj (via GitHub)" <gi...@apache.org>.
chickenlj merged PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476#issuecomment-2300128498
@chaokunyang PTAL
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476#discussion_r1724316099
##########
dubbo-serialization-extensions/dubbo-serialization-fury/src/main/java/org/apache/dubbo/common/serialize/fury/dubbo/FuryObjectInput.java:
##########
@@ -40,7 +40,7 @@ public FuryObjectInput(Fury fury, MemoryBuffer buffer, InputStream input) {
@Override
public Object readObject() {
- return fury.deserialize(new FuryInputStream(input, buffer.size()));
+ return fury.deserializeJavaObjectAndClass(new FuryInputStream(input, buffer.size()));
Review Comment:
Could we use `BlockedStreamUtils.deserialize(fury, output, obj);` and `BlockedStreamUtils.serialize(fury, output, obj);` instead?
`FuryInputStream` will buffer data internally for speed, which may read more data than needed
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "songxiaosheng (via GitHub)" <gi...@apache.org>.
songxiaosheng commented on PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476#issuecomment-2298681617
please fix conflict
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "yunshen-Lee (via GitHub)" <gi...@apache.org>.
yunshen-Lee commented on code in PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476#discussion_r1724435306
##########
dubbo-serialization-extensions/dubbo-serialization-fury/src/main/java/org/apache/dubbo/common/serialize/fury/dubbo/FuryObjectInput.java:
##########
@@ -40,7 +40,7 @@ public FuryObjectInput(Fury fury, MemoryBuffer buffer, InputStream input) {
@Override
public Object readObject() {
- return fury.deserialize(new FuryInputStream(input, buffer.size()));
+ return fury.deserializeJavaObjectAndClass(new FuryInputStream(input, buffer.size()));
Review Comment:
We should not always create a new FuryInputStream here, but instead use the same one. We should create a FuryInputStream when constructing FuryObjectInput
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "heliang666s (via GitHub)" <gi...@apache.org>.
heliang666s commented on PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476#issuecomment-2298698992
> please fix conflict
have done
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix fury bug [dubbo-spi-extensions]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #476:
URL: https://github.com/apache/dubbo-spi-extensions/pull/476#issuecomment-2299576606
## [Codecov](https://app.codecov.io/gh/apache/dubbo-spi-extensions/pull/476?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 0.00%. Comparing base [(`9b48975`)](https://app.codecov.io/gh/apache/dubbo-spi-extensions/commit/9b48975e2e8c5d996bc25319fe7c93f0e3582be0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`0fafa0e`)](https://app.codecov.io/gh/apache/dubbo-spi-extensions/commit/0fafa0e9e9d6e19fed5b56e0e8fbd8fa881acef1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 134 commits behind head on master.
> :exclamation: **Current head 0fafa0e differs from pull request most recent head 4303ed8**
>
> Please [upload](https://docs.codecov.com/docs/codecov-uploader) reports for the commit 4303ed8 to get more accurate results.
| [Files](https://app.codecov.io/gh/apache/dubbo-spi-extensions/pull/476?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...o/common/serialize/fury/dubbo/FuryObjectInput.java](https://app.codecov.io/gh/apache/dubbo-spi-extensions/pull/476?src=pr&el=tree&filepath=dubbo-serialization-extensions%2Fdubbo-serialization-fury%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fcommon%2Fserialize%2Ffury%2Fdubbo%2FFuryObjectInput.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tc2VyaWFsaXphdGlvbi1leHRlbnNpb25zL2R1YmJvLXNlcmlhbGl6YXRpb24tZnVyeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vY29tbW9uL3NlcmlhbGl6ZS9mdXJ5L2R1YmJvL0Z1cnlPYmplY3RJbnB1dC5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-spi-extensions/pull/476?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #476 +/- ##
===========================================
- Coverage 0.27% 0.00% -0.28%
===========================================
Files 299 521 +222
Lines 13730 17254 +3524
Branches 1737 2010 +273
===========================================
- Hits 38 0 -38
- Misses 13689 17254 +3565
+ Partials 3 0 -3
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo-spi-extensions/pull/476?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org