Skip to content

Commit

Permalink
Gateway: Add ETag to IPNS requests
Browse files Browse the repository at this point in the history 
Closes ipfs/go-ipfs#1818.

License: MIT
Signed-off-by: Johan Kiviniemi <[email protected]>
  • Loading branch information
@ion1
ion1 committed Nov 8, 2015
1 parent 96f4457 commit f44ebdb
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 10 deletions.
16 changes: 12 additions & 4 deletions core/corehttp/gateway_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,15 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
return
}

etag := gopath.Base(urlPath)
ndHash, err := nd.Key()
if err != nil {
internalWebError(w, err)
return
}

// ETag requires the quote marks:
// https://tools.ietf.org/html/rfc7232#section-2.3
etag := "\"" + ndHash.String() + "\""
if r.Header.Get("If-None-Match") == etag {
w.WriteHeader(http.StatusNotModified)
return
Expand Down Expand Up @@ -150,11 +158,10 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
// responses! The webError functions unset them automatically.
// TODO: Consider letting clients cache 404s. Are there cases where
// that would hurt?
setSuccessHeaders(w, ttl)
setSuccessHeaders(w, ttl, etag)

modtime := time.Now()
if strings.HasPrefix(urlPath, ipfsPathPrefix) {
w.Header().Set("Etag", etag)
// set modtime to a really long time ago, since files are immutable and should stay cached
modtime = time.Unix(1, 0)
}
Expand Down Expand Up @@ -457,8 +464,9 @@ func internalWebError(w http.ResponseWriter, err error) {
webErrorWithCode(w, "internalWebError", err, http.StatusInternalServerError)
}

func setSuccessHeaders(w http.ResponseWriter, ttl time.Duration) {
func setSuccessHeaders(w http.ResponseWriter, ttl time.Duration, etag string) {
w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(ttl.Seconds())))
w.Header().Set("ETag", etag)
}

func unsetSuccessHeaders(w http.ResponseWriter) {
Expand Down
42 changes: 36 additions & 6 deletions core/corehttp/gateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,15 @@ func TestGatewayGet(t *testing.T) {
path string
status int
ttl *time.Duration
etag string
text string
}{
{"localhost:5001", "/", http.StatusNotFound, nil, "404 page not found\n"},
{"localhost:5001", "/" + k, http.StatusNotFound, nil, "404 page not found\n"},
{"localhost:5001", "/ipfs/" + k, http.StatusOK, &immutableTTL, "fnord"},
{"localhost:5001", "/ipns/nxdomain.example.com", http.StatusBadRequest, nil, "Path Resolve error: " + namesys.ErrResolveFailed.Error()},
{"localhost:5001", "/ipns/example.com", http.StatusOK, &kTTL, "fnord"},
{"example.com", "/", http.StatusOK, &kTTL, "fnord"},
{"localhost:5001", "/", http.StatusNotFound, nil, "", "404 page not found\n"},
{"localhost:5001", "/" + k, http.StatusNotFound, nil, "", "404 page not found\n"},
{"localhost:5001", "/ipfs/" + k, http.StatusOK, &immutableTTL, k, "fnord"},
{"localhost:5001", "/ipns/nxdomain.example.com", http.StatusBadRequest, nil, "", "Path Resolve error: " + namesys.ErrResolveFailed.Error()},
{"localhost:5001", "/ipns/example.com", http.StatusOK, &kTTL, k, "fnord"},
{"example.com", "/", http.StatusOK, &kTTL, k, "fnord"},
} {
var c http.Client
r, err := http.NewRequest("GET", ts.URL+test.path, nil)
Expand All @@ -171,6 +172,7 @@ func TestGatewayGet(t *testing.T) {
continue
}
checkCacheControl(t, urlstr, resp, test.ttl)
checkETag(t, urlstr, resp, test.etag)
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Fatalf("error reading response from %s: %s", urlstr, err)
Expand Down Expand Up @@ -211,6 +213,10 @@ func TestIPNSHostnameRedirect(t *testing.T) {
if err != nil {
t.Fatal(err)
}
kFoo, err := dagn2.Key()
if err != nil {
t.Fatal(err)
}
t.Logf("k: %s\n", k)
kTTL := 10 * time.Minute
ns["/ipns/example.net"] = mockEntry{
Expand Down Expand Up @@ -240,7 +246,9 @@ func TestIPNSHostnameRedirect(t *testing.T) {
} else if hdr[0] != "/foo/" {
t.Errorf("location header is %v, expected /foo/", hdr[0])
}

checkCacheControl(t, "http://example.net/foo", res, &kTTL)
checkETag(t, "http://example.net/foo", res, kFoo.String())
}

func TestIPNSHostnameBacklinks(t *testing.T) {
Expand Down Expand Up @@ -277,6 +285,14 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
kFoo, err := dagn2.Key()
if err != nil {
t.Fatal(err)
}
kBar, err := dagn3.Key()
if err != nil {
t.Fatal(err)
}
t.Logf("k: %s\n", k)
kTTL := 10 * time.Minute
ns["/ipns/example.net"] = mockEntry{
Expand Down Expand Up @@ -315,6 +331,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
}

checkCacheControl(t, "http://example.net/foo/", res, &kTTL)
checkETag(t, "http://example.net/foo/", res, kFoo.String())

// make request to directory listing
req, err = http.NewRequest("GET", ts.URL, nil)
Expand Down Expand Up @@ -347,6 +364,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
}

checkCacheControl(t, "http://example.net/", res, &kTTL)
checkETag(t, "http://example.net/", res, k.String())

// make request to directory listing
req, err = http.NewRequest("GET", ts.URL+"/foo/bar/", nil)
Expand Down Expand Up @@ -379,6 +397,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
}

checkCacheControl(t, "http://example.net/foo/bar/", res, &kTTL)
checkETag(t, "http://example.net/foo/bar/", res, kBar.String())
}

func checkCacheControl(t *testing.T, urlstr string, resp *http.Response, ttl *time.Duration) {
Expand All @@ -391,3 +410,14 @@ func checkCacheControl(t *testing.T, urlstr string, resp *http.Response, ttl *ti
t.Errorf("unexpected Cache-Control header from %s: expected %q; got %q", urlstr, expCacheControl, cacheControl)
}
}

func checkETag(t *testing.T, urlstr string, resp *http.Response, expETagSansQuotes string) {
expETag := ""
if expETagSansQuotes != "" {
expETag = "\"" + expETagSansQuotes + "\""
}
etag := resp.Header.Get("ETag")
if etag != expETag {
t.Errorf("unexpected ETag header from %s: expected %q; got %q", urlstr, expETag, etag)
}
}
32 changes: 32 additions & 0 deletions test/sharness/t0110-gateway.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,26 @@ test_item_max_age() {
'
}

test_item_no_etag() {
_item="$1"

"$TEST_COMMAND" "$_item: has no ETag" '
test_get_header ETag actual.headers >actual.etag &&
test_must_be_empty actual.etag || test_fsh cat actual.headers
'
}

test_item_etag() {
_item="$1"
_etag="$2" # Without the surrounding quote marks

"$TEST_COMMAND" "$_item: has ETag" '
test_get_header ETag actual.headers >actual.etag &&
printf "\"%s\"\n" "$_etag" >expected.etag &&
test_cmp expected.etag actual.etag || test_fsh cat actual.headers
'
}

IMMUTABLE_TTL=$((10*365*24*60*60))
UNKNOWN_TTL=60

Expand All @@ -111,14 +131,17 @@ ITEM="GET IPFS path"
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH1" "200 OK"
test_item_output "$ITEM" "$DATA1"
test_item_max_age "$ITEM" "$IMMUTABLE_TTL"
test_item_etag "$ITEM" "$HASH1"

ITEM="GET IPFS path on API"
test_item_curl "$ITEM" "http://127.0.0.1:$apiport/ipfs/$HASH1" "403 Forbidden"
test_item_no_max_age "$ITEM"
test_item_no_etag "$ITEM"

ITEM="GET IPFS directory path"
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2" "200 OK"
test_item_max_age "$ITEM" "$IMMUTABLE_TTL"
test_item_etag "$ITEM" "$HASH2"
test_expect_success "$ITEM: output looks good" '
test_should_contain "Index of /ipfs/$HASH2" actual
'
Expand All @@ -127,10 +150,12 @@ ITEM="GET IPFS directory file"
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2/test" "200 OK"
test_item_output "$ITEM" "$DATA2/test"
test_item_max_age "$ITEM" "$IMMUTABLE_TTL"
test_item_etag "$ITEM" "$(ipfs add --only-hash -q "$DATA2/test")"

ITEM="GET IPFS directory path with index.html"
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2/has-index-html" "302 Found"
test_item_max_age "$ITEM" "$IMMUTABLE_TTL"
test_item_etag "$ITEM" "$(ipfs add --only-hash -r -q "$DATA2/has-index-html" | tail -n 1)"
test_expect_success "$ITEM: redirects to path/" '
test_get_header Location actual.headers >actual.location &&
printf "%s\n" "/ipfs/$HASH2/has-index-html/" >expected.location &&
Expand All @@ -141,10 +166,12 @@ ITEM="GET IPFS directory path/ with index.html"
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2/has-index-html/" "200 OK"
test_item_output "$ITEM" "$DATA2/has-index-html/index.html"
test_item_max_age "$ITEM" "$IMMUTABLE_TTL"
test_item_etag "$ITEM" "$(ipfs add --only-hash -r -q "$DATA2/has-index-html" | tail -n 1)"

ITEM="GET IPFS non-existent file"
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2/pleaseDontAddMe" "404 Not Found"
test_item_no_max_age "$ITEM"
test_item_no_etag "$ITEM"

TTL=10

Expand All @@ -155,6 +182,7 @@ test_expect_success_1941 "$ITEM: IPNS publish with TTL $TTL succeeds" '
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipns/$PEERID" "200 OK"
test_item_output "$ITEM" "$DATA1"
test_item_max_age "$ITEM" "$TTL"
test_item_etag "$ITEM" "$HASH1"

test_timer_start EXPIRY_TIMER "${TTL}s" # The cache entry has expired when this finishes.

Expand All @@ -167,6 +195,7 @@ test_expect_success_1941 "$ITEM: IPNS publish with default TTL succeeds" '
go-sleep 1s # Ensure the following max-age is strictly less than TTL.
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipns/$PEERID" "200 OK"
test_item_output "$ITEM" "$DATA1"
test_item_etag "$ITEM" "$HASH1"
test_expect_success_1941 "$ITEM: has Cache-Control max-age between 0 and $TTL" '
max_age="$(test_get_max_age actual.headers)" &&
test "$max_age" -gt 0 &&
Expand All @@ -188,14 +217,17 @@ ITEM="GET IPNS path again after cache expiry"
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipns/$PEERID/test" "200 OK"
test_item_output "$ITEM" "$DATA2/test"
test_item_max_age "$ITEM" "$UNKNOWN_TTL"
test_item_etag "$ITEM" "$(ipfs add --only-hash -q "$DATA2/test")"

ITEM="GET invalid IPFS path"
test_item_curl "$ITEM" "http://127.0.0.1:$port/ipfs/12345" "400 Bad Request"
test_item_no_max_age "$ITEM"
test_item_no_etag "$ITEM"

ITEM="GET invalid root path"
test_item_curl "$ITEM" "http://127.0.0.1:$port/12345" "404 Not Found"
test_item_no_max_age "$ITEM"
test_item_no_etag "$ITEM"

test_expect_success "GET /webui returns code expected" '
test_curl_resp_http_code "http://127.0.0.1:$apiport/webui" "HTTP/1.1 302 Found" "HTTP/1.1 301 Moved Permanently"
Expand Down

0 comments on commit f44ebdb

Please sign in to comment.