Skip to content

Conversation

@lzx404243
Copy link
Collaborator

@lzx404243 lzx404243 commented Jan 3, 2023

#5876 points out some mismatches between the documentation and the current behavior of ATS caching.

added coverage

Added AuTest coverage for caching behavior in the following areas to verify behavior:

  • no-store and no-cache cache-control directives in request/response
  • authorization headers(specifically response containing WWW-Authenticate)
  • cookie-related request/response(different configurations specifying which content type to cache)

documentation

The documentation is updated with the following changes:

  • Make it clear that ATS by default requires either Expires or max-age headers to cache
  • Make it clear that ATS by default ignores client no-cache and no-store directives
  • Removed Authorization header from the list that ATS by default doesn't cache(since it's cached per testing)
  • Make it clear that when proxy.config.http.cache.ignore_authentication is set to 1, the response containing the header would be cached
  • Listed out explicitly the pragma and cache-control directives that ATS ignores when ignore_client_no_cache or ignore_server_no_cache is enabled

AuTest failure

The newly added ResponseCacheControlIgnoredTest is failing, indicating that there are some issues with the proxy.config.http.cache.ignore_server_no_cache configuration. The test verifies that ATS ignores the no-cache and no-store in responses when asked to do so.

lzx404243 and others added 3 commits January 3, 2023 05:27
- no-store and no-cache cache-control directives in request/response
- authorization headers
- cookie-related request/response
@bneradt bneradt added this to the 10.0.0 milestone Jan 3, 2023
@bneradt
Copy link
Contributor

bneradt commented Jan 3, 2023

[approve ci]

Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I just have a few simple word-smithing suggestions for the docs.

Thanks for adding test coverage for these cases.

Make it clear that what ATS would ignore with ignore_client_no_cache and ignore_server_no_cache.
@lzx404243 lzx404243 marked this pull request as ready for review January 5, 2023 21:34
Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@bneradt
Copy link
Contributor

bneradt commented Jan 5, 2023

Marking for 9.2.x since this is a helpful doc and test-only change.

@bneradt bneradt merged commit ae05b17 into apache:master Jan 6, 2023
@lzx404243 lzx404243 deleted the issue-5876-cache-behavior branch January 6, 2023 19:42
@lzx404243 lzx404243 restored the issue-5876-cache-behavior branch January 10, 2023 17:21
zwoop pushed a commit that referenced this pull request Jan 10, 2023
* update documentation for certain cache behaviors

* remove trailing whitespaces

* reformated the doc

* Fixed typo

* Update doc for ignore_client_no_cache and ignore_server_no_cache

Make it clear that what ATS would ignore with ignore_client_no_cache and ignore_server_no_cache.

Co-authored-by: Zhengxi Li <zhengxi.li@yahooinc.com>
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
* asf/9.2.x:
  Updated ChangeLog
  Documentation-only changes from apache#9282 for 9.2.x (apache#9291)
  Fix a crash from a server early abort (apache#9287)
  Skip dns_host_down autest from 9.2.x branch (apache#9286)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants