kaazing / http2-cache.js

2 stars 11 forks source link

XHR2 caching responses with status code 403 #49

Closed tejaede closed 7 years ago

tejaede commented 7 years ago

Contour performs the following series of requests:

  1. https://testemops.pdc.org/hp_srv/services/hazards/t/json/get_hazards_count
    • No Authorization header. Responds with 403 status
  2. https://testemops.pdc.org/jwt/v1/auth/login2/
    • Retrieves token
  3. https://testemops.pdc.org/hp_srv/services/hazards/t/json/get_hazards_count
    • Includes Authorization header.

Expected: Request in step 3 goes to the origin server & responds with status 200 Actual: Request in step 3 returns cached 403 response.

My understanding of the HTTP spec is that 403 responses should not be cached.

From https://tools.ietf.org/html/rfc7231#section-6.1: Responses with status codes that are defined as cacheable by default (e.g., 200, 203, 204, 206, 300, 301, 404, 405, 410, 414, and 501 in this specification

Also, I tried to send "cache-control: no-cache" with the request in step 3 and found that http2 did not honor it due to a direct reference on a Map. See https://github.com/kaazing/http2-cache.js/pull/48

tejaede commented 7 years ago

cc @dpwspoon @hthetiot

hthetiot commented 7 years ago

spec: https://github.com/kaazing/http2-cache.js/blob/develop/test/http-cache-test.js#L59

dpwspoon commented 7 years ago

My understanding of the HTTP spec is that 403 responses should not be cached.

Correct, Implementation wise in the cache java nukleus we have logic to only cache

public static final List<String> CACHEABLE_BY_DEFAULT_STATUS_CODES = unmodifiableList(
            asList("200", "203", "204", "206", "300", "301", "404", "405", "410", "414", "501"));

We might be missing negative tests for this, let me check now

hthetiot commented 7 years ago

@dpwspoon See #51 and #52 (bonus #50)

dpwspoon commented 7 years ago

merged all