On Wed, 2020-05-27 at 14:47 +0200, Osipov, Michael wrote:
> Am 2020-05-27 um 12:35 schrieb Mark Thomas:
> > On 27/05/2020 10:59, Osipov, Michael wrote:
> > > 
> > > 
> > > Am 2020-05-27 um 10:51 schrieb Mark Thomas:
> > > > On 22/05/2020 22:59, Osipov, Michael wrote:
> > > > 
> > > > <snip/>
> > > > 
> > > > > I found one issue with HttpClient and Tomcat via HTTP/1.1. I
> > > > > have
> > > > > decrypted the TLS traffic [1]. I can see that HttpClient
> > > > > sends the
> > > > > headers also with a 4 KiB large chunk of the ZIP file. In
> > > > > return a
> > > > > Tomcats send the 401 response with:
> > > > > > Keep-Alive: timeout=300
> > > > > > Connection: keep-alive
> > > > > 
> > > > > The client keeps sending in 8 KiB large blocks. After 2 134
> > > > > 016 written
> > > > > bytes there is a TLS alert: Close Notify. A few packets
> > > > > later: RST.
> > > > > 
> > > > > I would expect to see here: "Connection: close".
> > > > 
> > > > Why?
> > > > 
> > > > Does the client provide a content-length header and no
> > > > expectation? If
> > > > so, Tomcat could determine that the maxSwallowSize is going to
> > > > be
> > > > exceeded and close the connection early.
> > > 
> > > Yes, it does. CL present and not expectation. A transcript is
> > > here [1].
> > > We have already a working patch for the early response, also Oleg
> > > is
> > > already working on the early close not happening which is a bug
> > > in
> > > HttpClient.
> > > 
> > > As for the connection header: I do not understand why you assume
> > > the
> > > header is no necessary. Your reasoning is that if the server
> > > closes the
> > > physical connection before the client has spooled the entire body
> > > and it
> > > is imperative for the client to close the connection too regard
> > > of the
> > > presense of the header?
> > 
> > I didn't assume the header is not necessary. I asked for further
> > explanation from you of why you thought it was necessary. I also
> > asked
> > some questions relevant to a potential justification for adding the
> > header.
> 
> Sorry if I caused confusion or missed something.
> 
> This is my understanding from section 6.5 the last paragraph. The 
> justification will follow below.
> 
> > Tomcat has enough information to know that:
> > - it needs to return a 401
> > - the request body won't be read (by the application)
> > - the request body is larger than maxSwallowSize so once that many
> > bytes
> >    have been swallowed, Tomcat is going to close the connection
> > 
> > At this point I think we are heading outside of behaviour defined
> > by the
> > HTTP/1.1 specification. I haven't found language covering aborted
> > uploads.
> > 
> > RFC 7230, 6.6 states a "Connection: close" header SHOULD be sent if
> > Tomcat knows it is going to close the connection. I think that
> > applies
> > in this case so I'd be in favour of adding that behaviour.
> 
> That's the first paragraph, I agree. See also this example from the
> now 
> working HttpClient:
> 
> > 5921 [main] DEBUG
> > org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManag
> > er - ep-00000001: connected http-outgoing-1
> > 5921 [main] DEBUG
> > org.apache.hc.client5.http.impl.classic.InternalHttpClient - ep-
> > 00000001: endpoint connected
> > 5921 [main] DEBUG
> > org.apache.hc.client5.http.impl.classic.MainClientExec - ex-
> > 00000002: executing POST /content-dev/api/documents HTTP/1.1
> > 5921 [main] DEBUG
> > org.apache.hc.client5.http.impl.classic.InternalHttpClient - ep-
> > 00000001: start execution ex-00000002
> > 5921 [main] DEBUG
> > org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManag
> > er - ep-00000001: executing exchange ex-00000002 over http-
> > outgoing-1
> > 5921 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 >> POST /content-dev/api/documents HTTP/1.1
> > 5921 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 >> Accept-Encoding: gzip, x-gzip, deflate
> > 5921 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 >> Content-Length: 7664149
> > 5921 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 >> Content-Type: application/zip
> > 5921 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 >> Host: <hostname>:11111
> > 5921 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 >> Connection: keep-alive
> > 5921 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 >> User-Agent: Apache-HttpClient/5.0.1-SNAPSHOT
> > (Java/13.0.2)
> > 5949 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 << HTTP/1.1 401 
> > 5949 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 << WWW-Authenticate: Negotiate
> > 5949 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 << Content-Type: text/html;charset=utf-8
> > 5949 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 << Content-Language: en
> > 5949 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 << Content-Length: 437
> > 5949 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 << Date: Wed, 27 May 2020 12:15:29 GMT
> > 5949 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 << Keep-Alive: timeout=300
> > 5949 [main] DEBUG org.apache.hc.client5.http.headers - http-
> > outgoing-1 << Connection: keep-alive
> > 5952 [main] DEBUG
> > org.apache.hc.client5.http.impl.classic.MainClientExec - ex-
> > 00000002: connection can be kept alive for 300 SECONDS
> > 5954 [main] DEBUG
> > org.apache.hc.client5.http.impl.auth.HttpAuthenticator -
> > Authentication required
> > 5954 [main] DEBUG
> > org.apache.hc.client5.http.impl.auth.HttpAuthenticator -
> > <hostname>:11111 requested authentication
> > 5954 [main] DEBUG
> > org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy -
> > Authentication schemes in the order of preference: [Negotiate,
> > Kerberos, NTLM, Digest, Basic]
> > 5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy -
> > Challenge for Kerberos authentication scheme not available
> > 5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy -
> > Challenge for NTLM authentication scheme not available
> > 5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy -
> > Challenge for Digest authentication scheme not available
> > 5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy -
> > Challenge for Basic authentication scheme not available
> > 5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.auth.HttpAuthenticator - Selecting
> > authentication options
> > 5956 [main] WARN
> > org.apache.hc.client5.http.impl.auth.HttpAuthenticator - Missing
> > auth challenge
> > <!doctype html><html lang="en"><head><title>HTTP Status 401 –
> > Unauthorized</title><style type="text/css">body {font-
> > family:Tahoma,Arial,sans-serif;} h1, h2, h3, b
> > {color:white;background-color:#525D76;} h1 {font-size:22px;} h2
> > {font-size:16px;} h3 {font-size:14px;} p {font-size:12px;} a
> > {color:black;} .line {height:1px;background-
> > color:#525D76;border:none;}</style></head><body><h1>HTTP Status 401
> > – Unauthorized</h1></body></html>5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.classic.InternalHttpClient - ep-
> > 00000001: releasing valid endpoint
> > 5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManag
> > er - ep-00000001: releasing endpoint
> > 5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManag
> > er - ep-00000001: connection is not kept alive
> > 5956 [main] DEBUG
> > org.apache.hc.client5.http.impl.io.DefaultManagedHttpClientConnecti
> > on - http-outgoing-1: close connection GRACEFUL
> > 5980 [main] DEBUG
> > org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManag
> > er - ep-00000001: connection released [route: {s}->
> > https://<hostname>:11111][total available: 0; route allocated: 0 of
> > 5; total allocated: 0 of 25]
> 
> C: Connection: keep-alive
> S: knows that the connection will be closed, but still sends keep
> alive 
> information.
> While HttpClient ignores this piece of information because the 
> connection must closed anyway accoring to section 6.5, but other
> clients 
> might not be so "smart". curl also handles this correcly with 
> "Connection: keep-alive".
> 
> > This then opens up an interesting question of whether to bother
> > reading
> > *any* of the request body if Tomcat knows it is going to close the
> > connection before reading all of it. Based on what you have
> > observed,
> > would earlier closure of the connection (after a response with a
> > "Connection: close" header) create additional difficulties? Would
> > those
> > clients that currently read the response while Tomcat is swallowing
> > the
> > first 2MB of upload still read the response if Tomcat didn't
> > swallow any
> > of the request? I think you can test this with maxSwallowSize="0"
> 
> I will test this, but this heavily depends on the client, especially
> how 
> I/O is handled. Blocking or non-blocking. The ClassicHttpClient is 
> blocking and what we do now is to wait (with a small timeout) for
> the 
> early response after having sent off the headers. We cannot detect
> this 
> in flight (after the first byte of the body has been sent) [1] while 
> non-blocking clients (e.g., curl, AsyncHttpClient) can detect in-
> flight.
> I don't think that there is a silverbullet or whether maxSwallowSize 
> should be by default 0.
> 
> @Oleg, what you is your opinion here because you supplied the
> patches?
> 

Hi Michael

Based on my reading of the specification the server does not need to
send `Connection: close` response header _as long as_ it is fully
prepared to consume and discard the rest of the request message body
declared in `Content-Length` request header. It is however might be
cheaper and safer to discard the connection anyway, in which
case `Connection: close` response header should be included in the
early response message. 

Cheers

Oleg


> Michael
> 
> [1] 
> 
https://github.com/ok2c/httpcomponents-core/compare/344b79f858f11064ca859faad09751b3610ba631...39f6d69a87586c147dc080431d45d6d48acb2c80


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to