psf / psf/cachecontrol

Setting max-age on the response causes cachecontrol to fail with exception about 'expires' argument

Open
#262 7 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
505
Forks
149
Avg merge
2d 13h
Merged PRs (30d)
7

Description

Applies to cachecontrol 0.12.7 and probably 0.12.8

When we have a response with following header: 'Cache-Control': 'public, max-age=1'

I believe this may be related to this PR: https://github.com/ionrock/cachecontrol/pull/233

We get the following exception from cachecontrol:

TypeError: set() got an unexpected keyword argument 'expires'
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/local/lib/python3.9/site-packages/requests/sessions.py:555: in get
    return self.request('GET', url, **kwargs)
/usr/local/lib/python3.9/site-packages/requests/sessions.py:542: in request
    resp = self.send(prep, **send_kwargs)
/usr/local/lib/python3.9/site-packages/requests/sessions.py:697: in send
    r.content
/usr/local/lib/python3.9/site-packages/requests/models.py:836: in content
    self._content = b''.join(self.iter_content(CONTENT_CHUNK_SIZE)) or b''
/usr/local/lib/python3.9/site-packages/requests/models.py:758: in generate
    for chunk in self.raw.stream(chunk_size, decode_content=True):
/usr/local/lib/python3.9/site-packages/urllib3/response.py:576: in stream
    data = self.read(amt=amt, decode_content=decode_content)
/usr/local/lib/python3.9/site-packages/urllib3/response.py:519: in read
    data = self._fp.read(amt) if not fp_closed else b""
/usr/local/lib/python3.9/site-packages/cachecontrol/filewrapper.py:96: in read
    self._close()
/usr/local/lib/python3.9/site-packages/cachecontrol/filewrapper.py:76: in _close
    self.__callback(result)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <cachecontrol.controller.CacheController object at 0x7f30ba331640>
request = <PreparedRequest [GET]>
response = <urllib3.response.HTTPResponse object at 0x7f30ba3179a0>, body = b''
status_codes = None
    def cache_response(self, request, response, body=None, status_codes=None):
        """
        Algorithm for caching requests.
    
        This assumes a requests Response object.
        """
        # From httplib2: Don't cache 206's since we aren't going to
        #                handle byte range requests
        cacheable_status_codes = status_codes or self.cacheable_status_codes
        if response.status not in cacheable_status_codes:
            logger.debug(
                "Status code %s not in %s", response.status, cacheable_status_codes
            )
            return
    
        response_headers = CaseInsensitiveDict(response.headers)
    
        if 'date' in response_headers:
            date = calendar.timegm(
                parsedate_tz(response_headers['date'])
            )
        else:
            date = 0
    
        # If we've been given a body, our response has a Content-Length, that
        # Content-Length is valid then we can check to see if the body we've
        # been given matches the expected size, and if it doesn't we'll just
        # skip trying to cache it.
        if (
            body is not None
            and "content-length" in response_headers
            and response_headers["content-length"].isdigit()
            and int(response_headers["content-length"]) != len(body)
        ):
            return
    
        cc_req = self.parse_cache_control(request.headers)
        cc = self.parse_cache_control(response_headers)
    
        cache_url = self.cache_url(request.url)
        logger.debug('Updating cache with response from "%s"', cache_url)
    
        # Delete it from the cache if we happen to have it stored there
        no_store = False
        if "no-store" in cc:
            no_store = True
            logger.debug('Response header has "no-store"')
        if "no-store" in cc_req:
            no_store = True
            logger.debug('Request header has "no-store"')
        if no_store and self.cache.get(cache_url):
            logger.debug('Purging existing cache entry to honor "no-store"')
            self.cache.delete(cache_url)
        if no_store:
            return
    
        # https://tools.ietf.org/html/rfc7234#section-4.1:
        # A Vary header field-value of "*" always fails to match.
        # Storing such a response leads to a deserialization warning
        # during cache lookup and is not allowed to ever be served,
        # so storing it can be avoided.
        if "*" in response_headers.get("vary", ""):
            logger.debug('Response header has "Vary: *"')
            return
    
        # If we've been given an etag, then keep the response
        if self.cache_etags and "etag" in response_headers:
            expires_time = 0
            if response_headers.get('expires'):
                expires = parsedate_tz(response_headers['expires'])
                if expires is not None:
                    expires_time = calendar.timegm(expires) - date
    
            expires_time = max(expires_time, 14 * 86400)
    
            logger.debug('etag object cached for {0} seconds'.format(expires_time))
            logger.debug("Caching due to etag")
            self.cache.set(
                cache_url,
                self.serializer.dumps(request, response, body),
                expires=expires_time
            )
    
        # Add to the cache any permanent redirects. We do this before looking
        # that the Date headers.
        elif int(response.status) in PERMANENT_REDIRECT_STATUSES:
            logger.debug("Caching permanent redirect")
            self.cache.set(cache_url, self.serializer.dumps(request, response, b''))
    
        # Add to the cache if the response headers demand it. If there
        # is no date header then we can't do anything about expiring
        # the cache.
        elif "date" in response_headers:
            date = calendar.timegm(
                parsedate_tz(response_headers['date'])
            )
            # cache when there is a max-age > 0
            if "max-age" in cc and cc["max-age"] > 0:
                logger.debug("Caching b/c date exists and max-age > 0")
                expires_time = cc['max-age']
>               self.cache.set(
                    cache_url,
                    self.serializer.dumps(request, response, body),
                    expires=expires_time
                )
E               TypeError: set() got an unexpected keyword argument 'expires'
/usr/local/lib/python3.9/site-packages/cachecontrol/controller.py:354: TypeError

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in cachecontrol/controller.py at CacheController.cache_response, following the max-age path shown in the traceback and comparing the cache.set call with the cache implementation. Reproduce the failure with a response containing Cache-Control: public, max-age=1, then add coverage so the response is cached without the expires TypeError.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.