New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static files served by 'tools.staticdir' cause decompression errors in browsers when both 'tools.gzip' and 'tools.caching' are on, but only when served from the cache. #1190

Closed
bb-migration opened this Issue Dec 5, 2012 · 7 comments

Comments

4 participants
@bb-migration

bb-migration commented Dec 5, 2012

Originally reported by: Michiel Overtoom (Bitbucket: MichielOvertoom, GitHub: @motoom)


I think I found a bug when both 'tools.gzip' and 'tools.caching' are on, for static files served by 'tools.staticdir'. Static content is placed uncompressed in the cache, but incorrectly marked as compressed, and the
result is that only the first serving is send correctly compressed to the client. Subsequent requests cause errors in the browser.

By the way, no exceptions are thrown by CherryPy.

I have attached a small test program which demonstrates this problem. Reloading the static file in the browser should produce the problem.


@bb-migration

This comment has been minimized.

Show comment
Hide comment
@bb-migration

bb-migration Jan 3, 2015

Original comment by George Weinberg (Bitbucket: gweinberg, GitHub: Unknown):


Based on my tests, it looks like the proper information is stored in the cache and retriived by cache.get
I think the problem is in the tee_ouput function in lib/caching.py, It ends with
response.body = tee(response.body)
but the internal function "tee" is returning None, so the response.body is set to None.
replacing the line with
tee(response.body)
without the assignment seems to fix the problem.

bb-migration commented Jan 3, 2015

Original comment by George Weinberg (Bitbucket: gweinberg, GitHub: Unknown):


Based on my tests, it looks like the proper information is stored in the cache and retriived by cache.get
I think the problem is in the tee_ouput function in lib/caching.py, It ends with
response.body = tee(response.body)
but the internal function "tee" is returning None, so the response.body is set to None.
replacing the line with
tee(response.body)
without the assignment seems to fix the problem.

@hexaclock

This comment has been minimized.

Show comment
Hide comment
@hexaclock

hexaclock Aug 14, 2017

Contributor

I just encountered this issue in CherryPy 10. The fix by George seems to work, but I'm not sure what implications/side effects it may have.

Contributor

hexaclock commented Aug 14, 2017

I just encountered this issue in CherryPy 10. The fix by George seems to work, but I'm not sure what implications/side effects it may have.

@webknjaz

This comment has been minimized.

Show comment
Hide comment
@webknjaz

webknjaz Aug 14, 2017

Member

In would be nice to get a PR for this ;)

Member

webknjaz commented Aug 14, 2017

In would be nice to get a PR for this ;)

@motoom

This comment has been minimized.

Show comment
Hide comment
@motoom

motoom Aug 16, 2017

Contributor

I did some archeology on my harddisk and found the original code I used to demonstrate this problem, years ago. Maybe it can be turned into a test? ;-)

import cherrypy
import time
import os
import requests   
import sys

'''
Demonstrates the "Content Encoding Error" given by the webbrowser
when getting a compressed static file from the cache.

There is a bug when both 'tools.gzip' and 'tools.caching' are on, 
for static files served by 'tools.staticdir'. Static content is placed 
uncompressed in the cache, but incorrectly marked as compressed, and the
result is that only the first serving is send correctly compressed to 
the client. Subsequent requests cause errors in the browser.
'''

class Root(object):
    @cherrypy.expose
    def index(self):
        print cherrypy.request.headers
        return """Static file: <a href="/static/SOMEFILE.TXT">SOMEFILE.TXT</a>, or <a href="large">large page</a>"""

    @cherrypy.expose
    def large(self):
        return "This is a large page. " * 1000

    @cherrypy.expose
    def clear(self):
        cherrypy._cache.clear()
        return "Cache cleared"
   
# Adjust for your situation.
if "darwin" in sys.platform:
    staticdirectory = "/Users/user/prj/python/cherrypy-examples/"
else:
    staticdirectory = "/home/user/prj/python/cherrypy-examples/"
    
conf = {
    "/": {         
        "response.timeout": 3600, # because of lengthy 'pudb' debug sessions
        "tools.gzip.on": True,        
        "tools.gzip.debug": True,        
        "tools.caching.on": True,
        "tools.caching.debug": True,
        },
    "/static": {
        "tools.staticdir.on": True,
        "tools.staticdir.debug": True,
        "tools.staticdir.dir": staticdirectory, 
        }
    }

if not os.path.exists(conf["/static"]["tools.staticdir.dir"]):
    raise Exception("You should adjust the static directory in conf")

# Create a test file.
f = open("static-test-file.txt", "w")
f.write("Abracadabra abracadabra abracadabra abracadabra")
f.close()

root = Root()
cherrypy.tree.mount(root, "/", conf)
cherrypy.engine.start()

'''How to demonstrate the problem from the commandline:
         
# curl --compressed http://127.0.0.1:8080/static/static-test-file.txt     
# curl -i --raw --compressed http://127.0.0.1:8080/static/static-test-file.txt  (for more insight)

Example run:

$ curl --compressed http://127.0.0.1:8080/static/static-test-file.txt
Abracadabra abracadabra abracadabra abracadabra

$ curl --compressed http://127.0.0.1:8080/static/static-test-file.txt
curl: (61) Error while processing content unencoding: invalid stored block lengths
'''


'''
# Response size should be the same for every request. But it isn't!
for i in range(2):
    r = requests.get("http://127.0.0.1:8080/static/static-test-file.txt",
                    headers={"Accept-Encoding": "gzip"})
    rawdata = r.raw.read()
    print "Response has %d bytes: %r" % (len(rawdata), rawdata)
'''

cherrypy.engine.block()
cherrypy.engine.exit()
Contributor

motoom commented Aug 16, 2017

I did some archeology on my harddisk and found the original code I used to demonstrate this problem, years ago. Maybe it can be turned into a test? ;-)

import cherrypy
import time
import os
import requests   
import sys

'''
Demonstrates the "Content Encoding Error" given by the webbrowser
when getting a compressed static file from the cache.

There is a bug when both 'tools.gzip' and 'tools.caching' are on, 
for static files served by 'tools.staticdir'. Static content is placed 
uncompressed in the cache, but incorrectly marked as compressed, and the
result is that only the first serving is send correctly compressed to 
the client. Subsequent requests cause errors in the browser.
'''

class Root(object):
    @cherrypy.expose
    def index(self):
        print cherrypy.request.headers
        return """Static file: <a href="/static/SOMEFILE.TXT">SOMEFILE.TXT</a>, or <a href="large">large page</a>"""

    @cherrypy.expose
    def large(self):
        return "This is a large page. " * 1000

    @cherrypy.expose
    def clear(self):
        cherrypy._cache.clear()
        return "Cache cleared"
   
# Adjust for your situation.
if "darwin" in sys.platform:
    staticdirectory = "/Users/user/prj/python/cherrypy-examples/"
else:
    staticdirectory = "/home/user/prj/python/cherrypy-examples/"
    
conf = {
    "/": {         
        "response.timeout": 3600, # because of lengthy 'pudb' debug sessions
        "tools.gzip.on": True,        
        "tools.gzip.debug": True,        
        "tools.caching.on": True,
        "tools.caching.debug": True,
        },
    "/static": {
        "tools.staticdir.on": True,
        "tools.staticdir.debug": True,
        "tools.staticdir.dir": staticdirectory, 
        }
    }

if not os.path.exists(conf["/static"]["tools.staticdir.dir"]):
    raise Exception("You should adjust the static directory in conf")

# Create a test file.
f = open("static-test-file.txt", "w")
f.write("Abracadabra abracadabra abracadabra abracadabra")
f.close()

root = Root()
cherrypy.tree.mount(root, "/", conf)
cherrypy.engine.start()

'''How to demonstrate the problem from the commandline:
         
# curl --compressed http://127.0.0.1:8080/static/static-test-file.txt     
# curl -i --raw --compressed http://127.0.0.1:8080/static/static-test-file.txt  (for more insight)

Example run:

$ curl --compressed http://127.0.0.1:8080/static/static-test-file.txt
Abracadabra abracadabra abracadabra abracadabra

$ curl --compressed http://127.0.0.1:8080/static/static-test-file.txt
curl: (61) Error while processing content unencoding: invalid stored block lengths
'''


'''
# Response size should be the same for every request. But it isn't!
for i in range(2):
    r = requests.get("http://127.0.0.1:8080/static/static-test-file.txt",
                    headers={"Accept-Encoding": "gzip"})
    rawdata = r.raw.read()
    print "Response has %d bytes: %r" % (len(rawdata), rawdata)
'''

cherrypy.engine.block()
cherrypy.engine.exit()
@webknjaz

This comment has been minimized.

Show comment
Hide comment
@webknjaz

webknjaz Aug 16, 2017

Member

@motoom thanks for checking this, according to @hexaclock's PR #1620, that patch was incorrect. What do you think about his proposed solution?

Member

webknjaz commented Aug 16, 2017

@motoom thanks for checking this, according to @hexaclock's PR #1620, that patch was incorrect. What do you think about his proposed solution?

@hexaclock

This comment has been minimized.

Show comment
Hide comment
@hexaclock

hexaclock Aug 16, 2017

Contributor

To clarify - I think the patch was incorrect because it ended up causing some tests to fail, and I don't believe it fixed the issue in all of the cases I had in my project.

Contributor

hexaclock commented Aug 16, 2017

To clarify - I think the patch was incorrect because it ended up causing some tests to fail, and I don't believe it fixed the issue in all of the cases I had in my project.

@webknjaz

This comment has been minimized.

Show comment
Hide comment
@webknjaz

webknjaz Aug 16, 2017

Member

Which means, before merging the PR we need to be 100% sure about its correctness, so that it won't cause new bugs :)

Member

webknjaz commented Aug 16, 2017

Which means, before merging the PR we need to be 100% sure about its correctness, so that it won't cause new bugs :)

@webknjaz webknjaz closed this in b7bf2a2 Sep 7, 2017

webknjaz added a commit that referenced this issue Sep 7, 2017

Fix gzip, caching and staticdir tools integration
Makes cache of gzipped content valid

Closes #1190
Ref #1620 (PR by @hexaclock)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment