You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "finefuture (via GitHub)" <gi...@apache.org> on 2024/05/28 09:33:33 UTC
[PR] Triple http limiting the size of the HTTP request and response [dubbo]
finefuture opened a new pull request, #14246:
URL: https://github.com/apache/dubbo/pull/14246
## What is the purpose of the change
refer: https://github.com/apache/dubbo/issues/14068
1. When an HttpStatusException occurs during serialization, It does not need to be wrapped as an EncodeException
2. Use LimitedByteBufOutputStream to limit the size of the write data. Throws an HttpOverPayloadException when the limit is exceeded
3. Fix some typos
For example:
```java
tripleProtocol.setTriple(TripleBuilder.newBuilder()
.maxBodySize(50)
.maxResponseBodySize(100)
.build());
```
Success result:
Request body limit result:
Response body limit result:
## Brief changelog
## Verifying this change
<!-- Follow this checklist to help us incorporate your contribution quickly and easily: -->
## Checklist
- [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.
- [ ] 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.
- [ ] Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
- [ ] 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.
- [ ] Add some description to [dubbo-website](https://github.com/apache/dubbo-website) project if you are requesting to add a feature.
- [ ] GitHub Actions works fine on your own branch.
- [ ] 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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2146479421
In http2:
1. In case of serialization failure, add tri-status and tri-message in trailers to indicate the reason for failure, and keep the header code as 200, or serialize first and then send the header frame for unary calls?
2. Can content-length be placed in trailers? Is it necessary to place it? Or provide configuration to write content-length?
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2134855899
## [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolutions=WONTFIX)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_coverage&view=list)
 [2.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246)
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2145060449
> I think h2 respone also need limit respone size.
HTTP/2 also limits the response body size and uses tri-status and tri-message identifiers.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2154732822
LGTM.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1622375754
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/exception/HttpOverPayloadException.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.remoting.http12.exception;
+
+public class HttpOverPayloadException extends HttpStatusException {
+
+ public HttpOverPayloadException(String message) {
+ super(500, message);
Review Comment:
Yes, for request body we can you 413, respone should use 500
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1624336130
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/h1/LimitedByteBufOutputStream.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.remoting.http12.h1;
+
+import org.apache.dubbo.remoting.http12.exception.HttpOverPayloadException;
+
+import java.io.IOException;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufOutputStream;
+
+public class LimitedByteBufOutputStream extends ByteBufOutputStream {
+
+ private final int capacity;
+
+ private int writeIndex;
+
+ public LimitedByteBufOutputStream(ByteBuf byteBuf, int capacity) {
+ super(byteBuf);
+ this.capacity = capacity == 0 ? Integer.MAX_VALUE : capacity;
+ this.writeIndex = 0;
+ }
+
+ @Override
+ public void write(int b) throws IOException {
+ ensureCapacity(1);
+ super.write(b);
+ }
+
+ @Override
+ public void write(byte[] b) throws IOException {
+ ensureCapacity(b.length);
+ super.write(b);
+ }
+
+ @Override
+ public void write(byte[] b, int off, int len) throws IOException {
+ ensureCapacity(len);
+ super.write(b, off, len);
+ }
+
+ private void ensureCapacity(int len) {
+ writeIndex += len;
Review Comment:
Optimized.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1621921784
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/exception/HttpOverPayloadException.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.remoting.http12.exception;
+
+public class HttpOverPayloadException extends HttpStatusException {
+
+ public HttpOverPayloadException(String message) {
+ super(500, message);
Review Comment:
Should be 413 here.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/413
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1627413233
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/h1/LimitedByteBufOutputStream.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.remoting.http12.h1;
Review Comment:
H2 is also used, it’s better to use a common package.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2146524985
Http2 no longer requires a content-length header, but some clients may use content-length to determine whether the data stream is complete. Therefore, refer to some common implementations in the industry to decide whether to add a content-length header to the http2 trailers, such as spring boot, tomcat, etc.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1622380139
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/h1/Http1ServerUnaryChannelObserver.java:
##########
@@ -55,4 +62,13 @@ protected void preMetadata(HttpMetadata httpMetadata, HttpOutputMessage outputMe
httpMetadata.headers().set(HttpHeaderNames.CONTENT_LENGTH.getName(), String.valueOf(contentLength));
}
}
+
+ private void handleOverPayload(HttpOverPayloadException overPayloadException) {
+ HttpMetadata httpMetadata = encodeHttpMetadata();
+ List<String> overPayloadStatus =
Review Comment:
Why status value is an array?
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2142059788
I thinkd h2 respone also need limit respone size.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1627414327
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/h1/LimitedByteBufOutputStream.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.remoting.http12.h1;
Review Comment:
Okay, I'll fix it.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "EarthChen (via GitHub)" <gi...@apache.org>.
EarthChen merged PR #14246:
URL: https://github.com/apache/dubbo/pull/14246
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2134768821
@oxsean 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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1622380139
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/h1/Http1ServerUnaryChannelObserver.java:
##########
@@ -55,4 +62,13 @@ protected void preMetadata(HttpMetadata httpMetadata, HttpOutputMessage outputMe
httpMetadata.headers().set(HttpHeaderNames.CONTENT_LENGTH.getName(), String.valueOf(contentLength));
}
}
+
+ private void handleOverPayload(HttpOverPayloadException overPayloadException) {
+ HttpMetadata httpMetadata = encodeHttpMetadata();
+ List<String> overPayloadStatus =
Review Comment:
Why status value is an list?
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2149434915
## [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolutions=WONTFIX)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_coverage&view=list)
 [1.4% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246)
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1621005549
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/h1/LimitedByteBufOutputStream.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.remoting.http12.h1;
+
+import org.apache.dubbo.remoting.http12.exception.HttpOverPayloadException;
+
+import java.io.IOException;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufOutputStream;
+
+public class LimitedByteBufOutputStream extends ByteBufOutputStream {
+
+ private final int capacity;
+
+ private int writeIndex;
+
+ public LimitedByteBufOutputStream(ByteBuf byteBuf, int capacity) {
+ super(byteBuf);
+ this.capacity = capacity == 0 ? Integer.MAX_VALUE : capacity;
+ this.writeIndex = 0;
+ }
+
+ @Override
+ public void write(int b) throws IOException {
+ ensureCapacity(1);
+ super.write(b);
+ }
+
+ @Override
+ public void write(byte[] b) throws IOException {
+ ensureCapacity(b.length);
+ super.write(b);
+ }
+
+ @Override
+ public void write(byte[] b, int off, int len) throws IOException {
+ ensureCapacity(len);
+ super.write(b, off, len);
+ }
+
+ private void ensureCapacity(int len) {
+ writeIndex += len;
Review Comment:
Can we use if(writtenBytes()+len>capacity) and remove writeIndex?
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/exception/HttpOverPayloadException.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.remoting.http12.exception;
+
+public class HttpOverPayloadException extends HttpStatusException {
+
+ public HttpOverPayloadException(String message) {
+ super(500, message);
Review Comment:
Yes, for request body we can yhou 413, respone should use 500
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1621945002
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/exception/HttpOverPayloadException.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.remoting.http12.exception;
+
+public class HttpOverPayloadException extends HttpStatusException {
+
+ public HttpOverPayloadException(String message) {
+ super(500, message);
Review Comment:
4xx is a client error, 5xx is a server error, and response too large is a server error, I don't think 413 can be used.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1627412742
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHeaderEnum.java:
##########
@@ -45,8 +45,7 @@ public enum TripleHeaderEnum {
SERVICE_GROUP("tri-service-group"),
SERVICE_TIMEOUT("tri-service-timeout"),
TRI_HEADER_CONVERT("tri-header-convert"),
- TRI_EXCEPTION_CODE("tri-exception-code"),
- ;
+ TRI_EXCEPTION_CODE("tri-exception-code");
Review Comment:
I see this seems to be used in TripleClientStream.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1627426889
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHeaderEnum.java:
##########
@@ -45,8 +45,7 @@ public enum TripleHeaderEnum {
SERVICE_GROUP("tri-service-group"),
SERVICE_TIMEOUT("tri-service-timeout"),
TRI_HEADER_CONVERT("tri-header-convert"),
- TRI_EXCEPTION_CODE("tri-exception-code"),
- ;
+ TRI_EXCEPTION_CODE("tri-exception-code");
Review Comment:
Ok, didn't realize it's a code format 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: 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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2139505182
## [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolutions=WONTFIX)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_coverage&view=list)
 [2.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246)
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2146481407
A problem encountered is that errors may occur during serialization. At this time, if the header is written first, the status code will be 200, but the result is failure.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2160477324
## [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolutions=WONTFIX)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_coverage&view=list)
 [1.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246)
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2159653394
@EarthChen 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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1624333824
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/h1/Http1ServerUnaryChannelObserver.java:
##########
@@ -55,4 +62,13 @@ protected void preMetadata(HttpMetadata httpMetadata, HttpOutputMessage outputMe
httpMetadata.headers().set(HttpHeaderNames.CONTENT_LENGTH.getName(), String.valueOf(contentLength));
}
}
+
+ private void handleOverPayload(HttpOverPayloadException overPayloadException) {
+ HttpMetadata httpMetadata = encodeHttpMetadata();
+ List<String> overPayloadStatus =
Review Comment:
Fixed.
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2145150346
## [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=14246&resolutions=WONTFIX)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=14246&resolved=false&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_coverage&view=list)
 [1.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=14246&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=14246)
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "finefuture (via GitHub)" <gi...@apache.org>.
finefuture commented on PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#issuecomment-2149365862
New results
http2:
http1:
--
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] Triple http limiting the size of the HTTP request and response [dubbo]
Posted by "oxsean (via GitHub)" <gi...@apache.org>.
oxsean commented on code in PR #14246:
URL: https://github.com/apache/dubbo/pull/14246#discussion_r1627406750
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHeaderEnum.java:
##########
@@ -45,8 +45,7 @@ public enum TripleHeaderEnum {
SERVICE_GROUP("tri-service-group"),
SERVICE_TIMEOUT("tri-service-timeout"),
TRI_HEADER_CONVERT("tri-header-convert"),
- TRI_EXCEPTION_CODE("tri-exception-code"),
- ;
+ TRI_EXCEPTION_CODE("tri-exception-code");
Review Comment:
Do we still need this?
--
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