You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by mi...@apache.org on 2019/03/31 08:03:58 UTC
[tomcat] branch wrong-http2-version created (now 0e1de0d)
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a change to branch wrong-http2-version
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
at 0e1de0d Fix wrong protocol version usage
This branch includes the following new commits:
new 0e1de0d Fix wrong protocol version usage
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 01/01: Fix wrong protocol version usage
Posted by Michael Osipov <mi...@apache.org>.
Am 2019-04-02 um 04:53 schrieb Christopher Schultz:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Michael,
>
> On 3/31/19 04:03, michaelo@apache.org wrote:
>> This is an automated email from the ASF dual-hosted git
>> repository.
>>
>> michaelo pushed a commit to branch wrong-http2-version in
>> repository https://gitbox.apache.org/repos/asf/tomcat.git
>>
>> commit 0e1de0d34302cdea6b3c2a47b03dcca4c7e2f9b7 Author: Michael
>> Osipov <mi...@apache.org> AuthorDate: Sun Mar 31 10:03:29 2019
>> +0200
>>
>> Fix wrong protocol version usage
>>
>> When serving a HTTP/2 request the protocol version was set as
>> "HTTP/2.0" which does not exist. ---
>> java/org/apache/coyote/http2/Stream.java | 2 +- 1 file changed, 1
>> insertion(+), 1 deletion(-)
>>
>> diff --git a/java/org/apache/coyote/http2/Stream.java
>> b/java/org/apache/coyote/http2/Stream.java index 3e64329..437279a
>> 100644 --- a/java/org/apache/coyote/http2/Stream.java +++
>> b/java/org/apache/coyote/http2/Stream.java @@ -126,7 +126,7 @@
>> class Stream extends AbstractStream implements HeaderEmitter {
>> this.coyoteRequest.setSendfile(handler.hasAsyncIO() &&
>> handler.getProtocol().getUseSendfile());
>> this.coyoteResponse.setOutputBuffer(http2OutputBuffer);
>> this.coyoteRequest.setResponse(coyoteResponse); -
>> this.coyoteRequest.protocol().setString("HTTP/2.0"); +
>> this.coyoteRequest.protocol().setString("HTTP/2"); if
>> (this.coyoteRequest.getStartTime() < 0) {
>> this.coyoteRequest.setStartTime(System.currentTimeMillis()); }
>
> Does this all come down to what is shown in the log files and what is
> returned by request.getProtocol() for servlets to consume? It isn't an
> issue with an HTTP/2 response at all.
Correct,
> Looks like it should be "HTTP/2.0", based upon these references:
>
> RFC 7540 (HTTP/2)
> Status line contains neither protocol nor status text: only the
> numeric code.
> https://tools.ietf.org/html/rfc7540#section-3.1
>
> JavaEE javadoc for HttpServletRequest#getProtocol:
>
> "
> Returns the name and version of the protocol the request uses in the
> form protocol/majorVersion.minorVersion, for example, HTTP/1.1. For
> HTTP servlets, the value returned is the same as the value of the CGI
> variable SERVER_PROTOCOL.
> "
>
> If we want to change what HttpServletRequest.getProtocol returns, I
> think we need to get agreement from the Servlet EG about an exception
> for HTTP/2 because anyone reading the documentation for that method
> should expect a ".0" on the end of "HTTP/2".
This is of course an argumentation I cannot resist to agree, though the
documentatin of %r is wrong:
%r - First line of the request (method and request URI)
It misses the protocol. Also for the sake of consistency I'd like to see:
coyoteRequest.protocol().setString(Constants.HTTP_20);
I'll drop the PR.
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 01/01: Fix wrong protocol version usage
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Michael,
On 3/31/19 04:03, michaelo@apache.org wrote:
> This is an automated email from the ASF dual-hosted git
> repository.
>
> michaelo pushed a commit to branch wrong-http2-version in
> repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit 0e1de0d34302cdea6b3c2a47b03dcca4c7e2f9b7 Author: Michael
> Osipov <mi...@apache.org> AuthorDate: Sun Mar 31 10:03:29 2019
> +0200
>
> Fix wrong protocol version usage
>
> When serving a HTTP/2 request the protocol version was set as
> "HTTP/2.0" which does not exist. ---
> java/org/apache/coyote/http2/Stream.java | 2 +- 1 file changed, 1
> insertion(+), 1 deletion(-)
>
> diff --git a/java/org/apache/coyote/http2/Stream.java
> b/java/org/apache/coyote/http2/Stream.java index 3e64329..437279a
> 100644 --- a/java/org/apache/coyote/http2/Stream.java +++
> b/java/org/apache/coyote/http2/Stream.java @@ -126,7 +126,7 @@
> class Stream extends AbstractStream implements HeaderEmitter {
> this.coyoteRequest.setSendfile(handler.hasAsyncIO() &&
> handler.getProtocol().getUseSendfile());
> this.coyoteResponse.setOutputBuffer(http2OutputBuffer);
> this.coyoteRequest.setResponse(coyoteResponse); -
> this.coyoteRequest.protocol().setString("HTTP/2.0"); +
> this.coyoteRequest.protocol().setString("HTTP/2"); if
> (this.coyoteRequest.getStartTime() < 0) {
> this.coyoteRequest.setStartTime(System.currentTimeMillis()); }
Does this all come down to what is shown in the log files and what is
returned by request.getProtocol() for servlets to consume? It isn't an
issue with an HTTP/2 response at all.
Looks like it should be "HTTP/2.0", based upon these references:
RFC 2068 (HTTP/1.1)
Status line is definitely "HTTP" +"/" + DIGIT + "." + DIGIT
https://tools.ietf.org/html/rfc2068#section-3.1
RFC 2616 (HTTP/1.1)
Status line is definitely "HTTP" +"/" + DIGIT + "." + DIGIT
https://tools.ietf.org/html/rfc2616#section-3.1
RFC 7230 (HTTP/1.1, Message Syntax and Routing)
Status line is definitely "HTTP" +"/" + DIGIT + "." + DIGIT
https://tools.ietf.org/html/rfc7230#section-2.6
RFC 7540 (HTTP/2)
Status line contains neither protocol nor status text: only the
numeric code.
https://tools.ietf.org/html/rfc7540#section-3.1
JavaEE javadoc for HttpServletRequest#getProtocol:
"
Returns the name and version of the protocol the request uses in the
form protocol/majorVersion.minorVersion, for example, HTTP/1.1. For
HTTP servlets, the value returned is the same as the value of the CGI
variable SERVER_PROTOCOL.
"
If we want to change what HttpServletRequest.getProtocol returns, I
think we need to get agreement from the Servlet EG about an exception
for HTTP/2 because anyone reading the documentation for that method
should expect a ".0" on the end of "HTTP/2".
- -chris
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAlyizq0ACgkQHPApP6U8
pFiLnBAAjU4BkfFthMICpbSJ7E5PSY29spZn+AxOBZSuBn1Gdr1MWt7z7i/5x4oi
keUlv6J+Fu7Tbn1XLkNtkg0cV9rLvXHQYtlAFpGdsF9e+J6+yEralPJueLnsehRk
y2QhV71G6jkhDiWvZuNRctHNjHa7koM4oSwOAnJlJDPYouW4gAKHvnGKN20xs7Cd
rzNA+6edF87WLHZvQ9WqmpIppyIAz2phDfWKqItHx61is76lEhimYbdVllA7GJID
NZF8FbPclxVgkp/gkju8cNTEVWaqgsAYvZFnIV/5pCkGKaIzOPvgeTb90VWREQIf
3sKUdN4RrVNXsd8gV15WVoyGFsIGtRp/NicILDKB+AVI42+khfl/2JMWEYvIjTtm
gXR969Ew5DMFMv+7QTHao9dhoPJ3rYnemtU8ext2hREjCkl8AJhe3tYxWaMUNme8
/gqtwlw9oe7hte07Y7O6dbwg1Zml1/6ls4drXU30bBfCpJM7mPIWOpE4Wt88kUUj
Q7T2oUFzaM63w5Dw+V6F8OVcLUoqWze8rrWo2Z8KEc2fIbzJEZEkjnBVNImlTY2L
2mcobM30QGGUL2iyehaG8/zXY0foOje5mGoGN0CPu9EDXy64csKMtoe4wZ6kxo2/
1gBKbOsmkyQxSdbtqL1K4OcMoJY4g4t/XMTSINHc8wC4FuU1SzI=
=NrG4
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 01/01: Fix wrong protocol version usage
Posted by Rémy Maucherat <re...@apache.org>.
On Mon, Apr 1, 2019 at 9:48 AM Michael Osipov <mi...@apache.org> wrote:
> Am 2019-04-01 um 09:36 schrieb Rémy Maucherat:
> > On Sun, Mar 31, 2019 at 7:38 PM Michael Osipov <mi...@apache.org>
> wrote:
> >
> >> Am 2019-03-31 um 14:50 schrieb Konstantin Kolinko:
> >>> -1 (veto).
> >>> This was discussed several years ago, and the decision was to use
> >> "HTTP/2.0"
> >>> https://bz.apache.org/bugzilla/show_bug.cgi?id=58605
> >>
> >> I tripped over this when comparing access logs between Tomcat and
> >> Apache. Also, as you have noted, according to the RFC, it is HTTP/2, not
> >> HTTP/2.0. Only the PRI request uses HTTP/2.0 for compat reasons.
> >>
> >> I don't see a reason to keep an incorrect proto version. Just because
> >> Mozilla makes it wrong, it doens't mean we have to.
> >>
> >
> > The commit was vetoed with a reason given, so it should be reverted for
> > now, and then discussion may continue on the merits.
>
> The commit is not part of the master branch. I have intentionally
> created a PR. I don't like the CTR approach because it procudes mess in
> the first place on the commit log.
>
> I am happy if we can have that discussion on the mailing list or on the
> PR. For now, only Konstantin has objected, I'd like to hear other
> comitter's opinion on.
>
Correct, the branch name is mentioned in the commit message and I missed
it, I'm simply not used to it yet.
Rémy
Re: [tomcat] 01/01: Fix wrong protocol version usage
Posted by Michael Osipov <mi...@apache.org>.
Am 2019-04-01 um 09:36 schrieb Rémy Maucherat:
> On Sun, Mar 31, 2019 at 7:38 PM Michael Osipov <mi...@apache.org> wrote:
>
>> Am 2019-03-31 um 14:50 schrieb Konstantin Kolinko:
>>> -1 (veto).
>>> This was discussed several years ago, and the decision was to use
>> "HTTP/2.0"
>>> https://bz.apache.org/bugzilla/show_bug.cgi?id=58605
>>
>> I tripped over this when comparing access logs between Tomcat and
>> Apache. Also, as you have noted, according to the RFC, it is HTTP/2, not
>> HTTP/2.0. Only the PRI request uses HTTP/2.0 for compat reasons.
>>
>> I don't see a reason to keep an incorrect proto version. Just because
>> Mozilla makes it wrong, it doens't mean we have to.
>>
>
> The commit was vetoed with a reason given, so it should be reverted for
> now, and then discussion may continue on the merits.
The commit is not part of the master branch. I have intentionally
created a PR. I don't like the CTR approach because it procudes mess in
the first place on the commit log.
I am happy if we can have that discussion on the mailing list or on the
PR. For now, only Konstantin has objected, I'd like to hear other
comitter's opinion on.
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 01/01: Fix wrong protocol version usage
Posted by Rémy Maucherat <re...@apache.org>.
On Sun, Mar 31, 2019 at 7:38 PM Michael Osipov <mi...@apache.org> wrote:
> Am 2019-03-31 um 14:50 schrieb Konstantin Kolinko:
> > -1 (veto).
> > This was discussed several years ago, and the decision was to use
> "HTTP/2.0"
> > https://bz.apache.org/bugzilla/show_bug.cgi?id=58605
>
> I tripped over this when comparing access logs between Tomcat and
> Apache. Also, as you have noted, according to the RFC, it is HTTP/2, not
> HTTP/2.0. Only the PRI request uses HTTP/2.0 for compat reasons.
>
> I don't see a reason to keep an incorrect proto version. Just because
> Mozilla makes it wrong, it doens't mean we have to.
>
The commit was vetoed with a reason given, so it should be reverted for
now, and then discussion may continue on the merits.
Rémy
Re: [tomcat] 01/01: Fix wrong protocol version usage
Posted by Michael Osipov <mi...@apache.org>.
Am 2019-03-31 um 14:50 schrieb Konstantin Kolinko:
> -1 (veto).
> This was discussed several years ago, and the decision was to use "HTTP/2.0"
> https://bz.apache.org/bugzilla/show_bug.cgi?id=58605
I tripped over this when comparing access logs between Tomcat and
Apache. Also, as you have noted, according to the RFC, it is HTTP/2, not
HTTP/2.0. Only the PRI request uses HTTP/2.0 for compat reasons.
I don't see a reason to keep an incorrect proto version. Just because
Mozilla makes it wrong, it doens't mean we have to.
M
> вс, 31 мар. 2019 г. в 11:04, <mi...@apache.org>:
>
>>
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> michaelo pushed a commit to branch wrong-http2-version
>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>
>> commit 0e1de0d34302cdea6b3c2a47b03dcca4c7e2f9b7
>> Author: Michael Osipov <mi...@apache.org>
>> AuthorDate: Sun Mar 31 10:03:29 2019 +0200
>>
>> Fix wrong protocol version usage
>>
>> When serving a HTTP/2 request the protocol version was set as "HTTP/2.0"
>> which does not exist.
>> ---
>> java/org/apache/coyote/http2/Stream.java | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
>> index 3e64329..437279a 100644
>> --- a/java/org/apache/coyote/http2/Stream.java
>> +++ b/java/org/apache/coyote/http2/Stream.java
>> @@ -126,7 +126,7 @@ class Stream extends AbstractStream implements HeaderEmitter {
>> this.coyoteRequest.setSendfile(handler.hasAsyncIO() && handler.getProtocol().getUseSendfile());
>> this.coyoteResponse.setOutputBuffer(http2OutputBuffer);
>> this.coyoteRequest.setResponse(coyoteResponse);
>> - this.coyoteRequest.protocol().setString("HTTP/2.0");
>> + this.coyoteRequest.protocol().setString("HTTP/2");
>> if (this.coyoteRequest.getStartTime() < 0) {
>> this.coyoteRequest.setStartTime(System.currentTimeMillis());
>> }
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 01/01: Fix wrong protocol version usage
Posted by Konstantin Kolinko <kn...@gmail.com>.
-1 (veto).
This was discussed several years ago, and the decision was to use "HTTP/2.0"
https://bz.apache.org/bugzilla/show_bug.cgi?id=58605
вс, 31 мар. 2019 г. в 11:04, <mi...@apache.org>:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> michaelo pushed a commit to branch wrong-http2-version
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit 0e1de0d34302cdea6b3c2a47b03dcca4c7e2f9b7
> Author: Michael Osipov <mi...@apache.org>
> AuthorDate: Sun Mar 31 10:03:29 2019 +0200
>
> Fix wrong protocol version usage
>
> When serving a HTTP/2 request the protocol version was set as "HTTP/2.0"
> which does not exist.
> ---
> java/org/apache/coyote/http2/Stream.java | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
> index 3e64329..437279a 100644
> --- a/java/org/apache/coyote/http2/Stream.java
> +++ b/java/org/apache/coyote/http2/Stream.java
> @@ -126,7 +126,7 @@ class Stream extends AbstractStream implements HeaderEmitter {
> this.coyoteRequest.setSendfile(handler.hasAsyncIO() && handler.getProtocol().getUseSendfile());
> this.coyoteResponse.setOutputBuffer(http2OutputBuffer);
> this.coyoteRequest.setResponse(coyoteResponse);
> - this.coyoteRequest.protocol().setString("HTTP/2.0");
> + this.coyoteRequest.protocol().setString("HTTP/2");
> if (this.coyoteRequest.getStartTime() < 0) {
> this.coyoteRequest.setStartTime(System.currentTimeMillis());
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/01: Fix wrong protocol version usage
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch wrong-http2-version
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0e1de0d34302cdea6b3c2a47b03dcca4c7e2f9b7
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Sun Mar 31 10:03:29 2019 +0200
Fix wrong protocol version usage
When serving a HTTP/2 request the protocol version was set as "HTTP/2.0"
which does not exist.
---
java/org/apache/coyote/http2/Stream.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 3e64329..437279a 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -126,7 +126,7 @@ class Stream extends AbstractStream implements HeaderEmitter {
this.coyoteRequest.setSendfile(handler.hasAsyncIO() && handler.getProtocol().getUseSendfile());
this.coyoteResponse.setOutputBuffer(http2OutputBuffer);
this.coyoteRequest.setResponse(coyoteResponse);
- this.coyoteRequest.protocol().setString("HTTP/2.0");
+ this.coyoteRequest.protocol().setString("HTTP/2");
if (this.coyoteRequest.getStartTime() < 0) {
this.coyoteRequest.setStartTime(System.currentTimeMillis());
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org