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 2023/08/29 18:46:04 UTC
[GitHub] [httpcomponents-core] arturobernalg opened a new pull request, #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
arturobernalg opened a new pull request, #424:
URL: https://github.com/apache/httpcomponents-core/pull/424
This pull request introduces changes to ensure our HTTP handling aligns with the requirements set out in [RFC9110](https://www.rfc-editor.org/rfc/rfc9110). Specifically, we've added validation to ensure that [OPTIONS](https://www.rfc-editor.org/rfc/rfc9110#name-options) requests containing content also include a valid Content-Type header.
--
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
[GitHub] [httpcomponents-core] reschke commented on pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#issuecomment-1702432568
FWIW, as far as I can tell, this PR only checks that there is a Content-Type field value. It doesn't check its validity, right?
--
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
[GitHub] [httpcomponents-core] ok2c commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1311302155
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -127,4 +133,32 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
+ /**
+ * Validates the presence and format of the Content-Type header for an OPTIONS request.
+ *
+ * <p>
+ * According to the HTTP protocol, an {@link Method#OPTIONS} request that contains content must also have
+ * a valid Content-Type header. This method checks for the presence of the Content-Type header
+ * and attempts to parse it to ensure its validity. If the header is missing or cannot be parsed,
+ * a {@link ProtocolException} is thrown.
+ * </p>
+ *
+ * @param request The {@link HttpRequest} whose Content-Type header is to be validated. Must not be null.
+ * @throws ProtocolException If the Content-Type header is missing or its format is invalid.
+ */
+ public void validateOptionsContentType(final HttpRequest request) throws ProtocolException {
+ if (Method.OPTIONS.isSame(request.getMethod())) {
+ final Header header = request.getFirstHeader(HttpHeaders.CONTENT_TYPE);
+ if (header == null) {
+ throw new ProtocolException("OPTIONS request must have Content-Type header");
+ }
+ try {
Review Comment:
@arturobernalg Just because `Content-Type` can be parsed, does not mean it is valid. This check is expensive and almost useless. Please drop 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: 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
[GitHub] [httpcomponents-core] reschke commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1311309977
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -127,4 +133,32 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
+ /**
+ * Validates the presence and format of the Content-Type header for an OPTIONS request.
+ *
+ * <p>
+ * According to the HTTP protocol, an {@link Method#OPTIONS} request that contains content must also have
Review Comment:
FWIW, this requirement is not really specific to OPTIONS.
--
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
[GitHub] [httpcomponents-core] ok2c commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1312913134
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -110,21 +112,56 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
if (entity != null) {
+
Review Comment:
@arturobernalg Nooooooooo. Cmon! Some entities stream their content and their length is not known it advance! This is what you have `EntityDetails` for.
--
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
[GitHub] [httpcomponents-core] michael-o commented on pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#issuecomment-1702420779
Am I stupid or where do I see that the request actually contains content?
--
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
[GitHub] [httpcomponents-core] ok2c commented on pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#issuecomment-1702444761
> Am I stupid or where do I see that the request actually contains content?
@michael-o This is quite fair. @arturobernalg I think there should be a check there is actually an entity enclosed.
--
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
[GitHub] [httpcomponents-core] arturobernalg commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1312058180
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -127,4 +133,32 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
+ /**
+ * Validates the presence and format of the Content-Type header for an OPTIONS request.
+ *
+ * <p>
+ * According to the HTTP protocol, an {@link Method#OPTIONS} request that contains content must also have
+ * a valid Content-Type header. This method checks for the presence of the Content-Type header
+ * and attempts to parse it to ensure its validity. If the header is missing or cannot be parsed,
+ * a {@link ProtocolException} is thrown.
+ * </p>
+ *
+ * @param request The {@link HttpRequest} whose Content-Type header is to be validated. Must not be null.
+ * @throws ProtocolException If the Content-Type header is missing or its format is invalid.
+ */
+ public void validateOptionsContentType(final HttpRequest request) throws ProtocolException {
+ if (Method.OPTIONS.isSame(request.getMethod())) {
+ final Header header = request.getFirstHeader(HttpHeaders.CONTENT_TYPE);
+ if (header == null) {
+ throw new ProtocolException("OPTIONS request must have Content-Type header");
+ }
+ try {
Review Comment:
@ok2c
Removed.
--
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
[GitHub] [httpcomponents-core] arturobernalg commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1311925115
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -127,4 +133,32 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
+ /**
+ * Validates the presence and format of the Content-Type header for an OPTIONS request.
+ *
+ * <p>
+ * According to the HTTP protocol, an {@link Method#OPTIONS} request that contains content must also have
Review Comment:
@reschke
To ensure I understand correctly, are you suggesting that the validation for a valid Content-Type header when content is present should be applied to all HTTP methods and not just OPTIONS? If so, are there specific methods you have in mind that should undergo this validation? Could you please specify or guide me to the exact section or sections in the RFC where this requirement is detailed for other HTTP methods?
--
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
[GitHub] [httpcomponents-core] ok2c commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1312801174
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -127,4 +133,32 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
+ /**
+ * Validates the presence and format of the Content-Type header for an OPTIONS request.
+ *
+ * <p>
+ * According to the HTTP protocol, an {@link Method#OPTIONS} request that contains content must also have
Review Comment:
@reschke What you are saying makes no sense. @arturobernalg We cannot truly know what if `content-type` value is valid or not unless we maintain a list of vales we consider valid.
--
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
[GitHub] [httpcomponents-core] arturobernalg merged pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg merged PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424
--
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
[GitHub] [httpcomponents-core] reschke commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1312784977
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -127,4 +133,32 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
+ /**
+ * Validates the presence and format of the Content-Type header for an OPTIONS request.
+ *
+ * <p>
+ * According to the HTTP protocol, an {@link Method#OPTIONS} request that contains content must also have
Review Comment:
What I'm trying to say is that I see little value in accepting invalid Content-Type fields for other methods.
--
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
[GitHub] [httpcomponents-core] ok2c commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1312913134
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -110,21 +112,56 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
if (entity != null) {
+
Review Comment:
@arturobernalg Nooooooooo. Cmon! Some entities stream their content and their length is not known it advance! This is what you have `EntityDetails` for. Basically if it is non-null there is an enclosed entity.
--
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
[GitHub] [httpcomponents-core] arturobernalg commented on a diff in pull request #424: HTTPCORE-756 - Implement RFC9110 Content-Type Validation for OPTIONS Requests
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #424:
URL: https://github.com/apache/httpcomponents-core/pull/424#discussion_r1312949660
##########
httpcore5/src/main/java/org/apache/hc/core5/http/protocol/RequestContent.java:
##########
@@ -110,21 +112,56 @@ public void process(final HttpRequest request, final EntityDetails entity, final
}
}
if (entity != null) {
+
Review Comment:
@ok2c Corrected.
--
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