You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "arturobernalg (via GitHub)" <gi...@apache.org> on 2026/02/16 10:59:03 UTC
[PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
arturobernalg opened a new pull request, #620:
URL: https://github.com/apache/httpcomponents-core/pull/620
Reject any TE value other than trailers (including additional TE headers).
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg merged PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620#discussion_r2812136299
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java:
##########
@@ -75,18 +75,23 @@ public H2RequestConformance() {
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
- for (int i = 0; i < illegalHeaderNames.length; i++) {
- final String headerName = illegalHeaderNames[i];
- final Header header = request.getFirstHeader(headerName);
- if (header != null) {
- if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
- final String value = header.getValue();
- if (!"trailers".equalsIgnoreCase(value)) {
+
+ for (final String headerName : illegalHeaderNames) {
+ if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
Review Comment:
@arturobernalg This gets ugly. Use a flag to signal that a TE is present and move TE parsing out of the for loop. Use `MessageSupport` methods to efficiently parse list elements.
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java:
##########
@@ -75,18 +75,23 @@ public H2RequestConformance() {
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
- for (int i = 0; i < illegalHeaderNames.length; i++) {
- final String headerName = illegalHeaderNames[i];
- final Header header = request.getFirstHeader(headerName);
- if (header != null) {
- if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
- final String value = header.getValue();
- if (!"trailers".equalsIgnoreCase(value)) {
+
+ for (final String headerName : illegalHeaderNames) {
Review Comment:
@arturobernalg enhanced for loop creates an extra iterator, which is unnecessary Please keep the simple loop.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620#discussion_r2813534421
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java:
##########
@@ -75,20 +76,37 @@ public H2RequestConformance() {
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
+
+ boolean validateTe = false;
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
- final Header header = request.getFirstHeader(headerName);
- if (header != null) {
- if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
- final String value = header.getValue();
- if (!"trailers".equalsIgnoreCase(value)) {
- throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, value);
- }
- } else {
- throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
- }
+ if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
+ validateTe = true;
+ continue;
+ }
+ if (request.getFirstHeader(headerName) != null) {
+ throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
}
}
+
+ if (validateTe) {
+ validateTE(request);
+ }
+ }
+
+ private static void validateTE(final HttpRequest request) throws ProtocolException {
+ boolean sawAnyToken = false;
+ for (final Iterator<String> it = MessageSupport.iterateTokens(request, HttpHeaders.TE); it.hasNext(); ) {
+ final String token = it.next();
+ sawAnyToken = true;
+ if (!"trailers".equalsIgnoreCase(token)) {
+ throw new ProtocolException(
+ "Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, token);
+ }
+ }
+ if (!sawAnyToken && request.getFirstHeader(HttpHeaders.TE) != null) {
Review Comment:
You need some equivalent of sawAnyToken if you want to reject these cases:
- TE:
- TE:
- TE: , , (only separators / whitespace)
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620#issuecomment-3914971957
> @arturobernalg I would rewrite it as
>
> ```
> @Override
> public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
> throws HttpException, IOException {
> Args.notNull(request, "HTTP request");
> for (int i = 0; i < illegalHeaderNames.length; i++) {
> final String headerName = illegalHeaderNames[i];
> if (request.containsHeader(headerName)) {
> if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
> validateTE(request);
> } else {
> throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
> }
> }
> }
> }
>
> private static void validateTE(final HttpRequest request) throws ProtocolException {
> boolean sawAnyToken = false;
> boolean sawInvalidToken = false;
> for (final Iterator<String> it = MessageSupport.iterateTokens(request, HttpHeaders.TE); it.hasNext(); ) {
> final String token = it.next();
> sawAnyToken = true;
> if (!"trailers".equalsIgnoreCase(token)) {
> sawInvalidToken = true;
> break;
> }
> }
> if (sawInvalidToken || !sawAnyToken) {
> throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", HttpHeaders.TE);
> }
> }
> ```
@ok2c All right. I made the 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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620#discussion_r2813532674
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java:
##########
@@ -75,20 +76,37 @@ public H2RequestConformance() {
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
+
+ boolean validateTe = false;
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
- final Header header = request.getFirstHeader(headerName);
- if (header != null) {
- if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
- final String value = header.getValue();
- if (!"trailers".equalsIgnoreCase(value)) {
- throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, value);
- }
- } else {
- throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
- }
+ if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
+ validateTe = true;
+ continue;
+ }
+ if (request.getFirstHeader(headerName) != null) {
Review Comment:
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620#issuecomment-3914630016
@arturobernalg I would rewrite it as
```
@Override
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
if (request.containsHeader(headerName)) {
if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
validateTE(request);
} else {
throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
}
}
}
}
private static void validateTE(final HttpRequest request) throws ProtocolException {
boolean sawAnyToken = false;
boolean sawInvalidToken = false;
for (final Iterator<String> it = MessageSupport.iterateTokens(request, HttpHeaders.TE); it.hasNext(); ) {
final String token = it.next();
sawAnyToken = true;
if (!"trailers".equalsIgnoreCase(token)) {
sawInvalidToken = true;
break;
}
}
if (sawInvalidToken || !sawAnyToken) {
throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", HttpHeaders.TE);
}
}
```
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620#discussion_r2813284810
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java:
##########
@@ -75,20 +76,37 @@ public H2RequestConformance() {
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
+
+ boolean validateTe = false;
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
- final Header header = request.getFirstHeader(headerName);
- if (header != null) {
- if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
- final String value = header.getValue();
- if (!"trailers".equalsIgnoreCase(value)) {
- throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, value);
- }
- } else {
- throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
- }
+ if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
+ validateTe = true;
+ continue;
+ }
+ if (request.getFirstHeader(headerName) != null) {
Review Comment:
@arturobernalg Use `#containsHeader` here. It is a bit nicer.
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java:
##########
@@ -75,20 +76,37 @@ public H2RequestConformance() {
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
+
+ boolean validateTe = false;
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
- final Header header = request.getFirstHeader(headerName);
- if (header != null) {
- if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
- final String value = header.getValue();
- if (!"trailers".equalsIgnoreCase(value)) {
- throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, value);
- }
- } else {
- throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
- }
+ if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
+ validateTe = true;
+ continue;
+ }
+ if (request.getFirstHeader(headerName) != null) {
+ throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
}
}
+
+ if (validateTe) {
+ validateTE(request);
+ }
+ }
+
+ private static void validateTE(final HttpRequest request) throws ProtocolException {
+ boolean sawAnyToken = false;
+ for (final Iterator<String> it = MessageSupport.iterateTokens(request, HttpHeaders.TE); it.hasNext(); ) {
+ final String token = it.next();
+ sawAnyToken = true;
+ if (!"trailers".equalsIgnoreCase(token)) {
+ throw new ProtocolException(
+ "Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, token);
+ }
+ }
+ if (!sawAnyToken && request.getFirstHeader(HttpHeaders.TE) != null) {
Review Comment:
@arturobernalg This is bad. Why would you want to do a look-up on here, again? You do you need this line at all? If there has been no `ProtocolException` no exception at this point, everything is OK.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620#discussion_r2813583263
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java:
##########
@@ -75,20 +76,37 @@ public H2RequestConformance() {
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
+
+ boolean validateTe = false;
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
- final Header header = request.getFirstHeader(headerName);
- if (header != null) {
- if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
- final String value = header.getValue();
- if (!"trailers".equalsIgnoreCase(value)) {
- throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, value);
- }
- } else {
- throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
- }
+ if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
+ validateTe = true;
+ continue;
+ }
+ if (request.getFirstHeader(headerName) != null) {
+ throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
}
}
+
+ if (validateTe) {
+ validateTE(request);
+ }
+ }
+
+ private static void validateTE(final HttpRequest request) throws ProtocolException {
+ boolean sawAnyToken = false;
+ for (final Iterator<String> it = MessageSupport.iterateTokens(request, HttpHeaders.TE); it.hasNext(); ) {
+ final String token = it.next();
+ sawAnyToken = true;
+ if (!"trailers".equalsIgnoreCase(token)) {
+ throw new ProtocolException(
+ "Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, token);
+ }
+ }
+ if (!sawAnyToken && request.getFirstHeader(HttpHeaders.TE) != null) {
Review Comment:
@arturobernalg I do not see a problem with ignoring `TE: ` or `TE: `. 'TE: , , , ,' however will get correctly rejected.
If you want to eliminate the former two cases as well, I think there is a way of doing without resorting to an extra `#getFirstHeader` call.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Validate all TE header instances for HTTP/2 request conformance. [httpcomponents-core]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #620:
URL: https://github.com/apache/httpcomponents-core/pull/620#discussion_r2813583263
##########
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java:
##########
@@ -75,20 +76,37 @@ public H2RequestConformance() {
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext localContext)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");
+
+ boolean validateTe = false;
for (int i = 0; i < illegalHeaderNames.length; i++) {
final String headerName = illegalHeaderNames[i];
- final Header header = request.getFirstHeader(headerName);
- if (header != null) {
- if (headerName.equalsIgnoreCase(HttpHeaders.TE)) {
- final String value = header.getValue();
- if (!"trailers".equalsIgnoreCase(value)) {
- throw new ProtocolException("Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, value);
- }
- } else {
- throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
- }
+ if (HttpHeaders.TE.equalsIgnoreCase(headerName)) {
+ validateTe = true;
+ continue;
+ }
+ if (request.getFirstHeader(headerName) != null) {
+ throw new ProtocolException("Header '%s' is illegal for HTTP/2 messages", headerName);
}
}
+
+ if (validateTe) {
+ validateTE(request);
+ }
+ }
+
+ private static void validateTE(final HttpRequest request) throws ProtocolException {
+ boolean sawAnyToken = false;
+ for (final Iterator<String> it = MessageSupport.iterateTokens(request, HttpHeaders.TE); it.hasNext(); ) {
+ final String token = it.next();
+ sawAnyToken = true;
+ if (!"trailers".equalsIgnoreCase(token)) {
+ throw new ProtocolException(
+ "Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, token);
+ }
+ }
+ if (!sawAnyToken && request.getFirstHeader(HttpHeaders.TE) != null) {
Review Comment:
@arturobernalg I do not see a problem with ignoring `TE: ` or `TE: `. 'TE: , , , ,' however will get correctly rejected.
~~If you want to eliminate the former two cases as well, I think there is a way of doing without resorting to an extra `#getFirstHeader` call.~~
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org