Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

Bug 828888

Summary: Proxy should behave correctly when removing Transfer-Encoding: chunked
Product: [Community] Spacewalk Reporter: Miroslav Suchý <msuchy>
Component: Proxy ServerAssignee: Jan Pazdziora (Red Hat) <jpazdziora>
Status: CLOSED NOTABUG QA Contact: Red Hat Satellite QA List <satqe-list>
Severity: low Docs Contact:
Priority: low    
Version: 1.8CC: jpazdziora, ppisar
Target Milestone: ---   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 824140 Environment:
Last Closed: 2013-03-08 14:29:17 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 824140    
Bug Blocks: 1484117    

Description Miroslav Suchý 2012-06-05 14:29:36 UTC
+++ This bug was initially created as a clone of Bug #824140 +++
(In reply to comment #23)
> Created attachment 589246 [details]
> patch for chunk encoding

If I understand correctly, the patch removes the Transfer-Encoding header from the response.

If you remove the Transfer-Encoding and you do not specify Content-Length, the server must send "Connection: close" and close the TCP connection after each response. Please make sure this is true for your proxy implementation. So far, it seems so.

The patch does "k == 'transfer-encoding'". Please note the HTTP header names are case-insensitive. That means a server can send "Transfer-Encoding: chunked". Are you sure the == operator handles it?

--- Additional comment from msuchy on 2012-06-05 05:00:09 EDT ---

> Please note the HTTP header names are case-insensitive. That means a server can send "Transfer-Encoding: chunked". Are you sure the == operator handles it?

Yes. All headers are lowercased in rhnShared._get_header()

> If you remove the Transfer-Encoding and you do not specify Content-Length, the server must send "Connection: close" and close the TCP connection after each response.

I carefully read
 http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
bud did not find such requirement. Can you specify RFC which require it?

--- Additional comment from ppisar on 2012-06-05 05:32:38 EDT ---

(In reply to comment #25)
> > If you remove the Transfer-Encoding and you do not specify Content-Length,
> > the server must send "Connection: close" and close the TCP connection after
> > each response.
> 
> I carefully read
>  http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
> bud did not find such requirement. Can you specify RFC which require it?

"4.4. Message Length" last option which applies to current implementation mandates closing connection. And the "Connection: close" is defined in "14.10 Connection".

Comment 1 Jan Pazdziora (Red Hat) 2012-06-05 15:58:40 UTC
Why not just specifying the content-length?

Comment 2 Jan Pazdziora (Red Hat) 2013-03-08 14:01:05 UTC
So, looking at the current code:

proxy/proxy/rhnShared.py: _forwardHTTPHeaders
            if (k == 'transfer-encoding') and ('chunked' in v):
                log_debug(5, "Filtering header", k, v)
                continue

is called from

proxy/proxy/rhnShared.py: _forwardServer2Client

        self._forwardHTTPHeaders(bodyFd, self.req)

and that one is called only from

    def _clientCommo(self, status=apache.OK):
        """ Handler part 3
            Forward server's response to the client.
        """
        log_debug(1)

        try:
            self._forwardServer2Client()
        except IOError:
            # Raised by HTTP*connection.getresponse
            # Client closed connection on us, no need to mail out a traceback
            Traceback("SharedHandler._clientCommo", self.req, mail=0)
            return apache.HTTP_SERVICE_UNAVAILABLE

        # Close all open response contexts.
        self.responseContext.clear()

        return status

And that self.responseContext.clear seems to close all the filehandles eventually.

I assume the "So far, it seems so." still holds and this whole bugzilla was about checking that we don't miss anything. Closing as NOTABUG, please reopen if you disagree.

Comment 3 Eric Herget 2017-09-28 17:56:31 UTC
This BZ closed some time during 2.5, 2.6 or 2.7.  Adding to 2.7 tracking bug.