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 7, 2015
1 parent 7f5aa71 commit 6ee526f
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 22 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)
}
}
10 changes: 10 additions & 0 deletions test/sharness/lib/test-lib.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,16 @@ test_should_contain() {
fi
}

test_should_not_contain() {
test "$#" = 2 || error "bug in the test script: not 2 parameters to test_should_not_contain"
if grep -q "$1" "$2"
then
echo "'$2' should not contain '$1', but it does:"
cat "$2"
return 1
fi
}

test_str_contains() {
find=$1
shift
Expand Down
44 changes: 32 additions & 12 deletions test/sharness/t0110-gateway.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ test_item() {
_http_status="$3"
_expected_output_file="$4"
_max_age="$5"
_etag="$6" # Without the surrounding quote marks

"$TEST_COMMAND" "$_item: responds with $_http_status" '
test_run_curl "$_url" &&
Expand Down Expand Up @@ -87,6 +88,21 @@ test_item() {
'
;;
esac

case "$_etag" in
NO_ETAG)
"$TEST_COMMAND" "$_item: has no ETag" '
test_should_not_contain "^[Ee][Tt][Aa][Gg]: " actual.headers
'
;;
*)
# The go http package replaces ETag with Etag. Accept both. (HTTP
# header names are case-insensitive but ETag is the official name.)
"$TEST_COMMAND" "$_item: has ETag" '
test_should_contain "^E[Tt]ag: \"$_etag\".\$" actual.headers
'
;;
esac
}

IMMUTABLE_TTL=$((10*365*24*60*60))
Expand All @@ -111,35 +127,38 @@ test_expect_success "Generating test environment" '
'

test_item "GET IPFS path" "http://127.0.0.1:$port/ipfs/$HASH1" \
"200 OK" "$DATA1" "$IMMUTABLE_TTL"
"200 OK" "$DATA1" "$IMMUTABLE_TTL" "$HASH1"

test_item "GET IPFS path on API" "http://127.0.0.1:$apiport/ipfs/$HASH1" \
"403 Forbidden" IGNORE_OUTPUT NO_MAX_AGE
"403 Forbidden" IGNORE_OUTPUT NO_MAX_AGE NO_ETAG

ITEM="GET IPFS directory path"
test_item "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2" \
"200 OK" IGNORE_OUTPUT "$IMMUTABLE_TTL"
"200 OK" IGNORE_OUTPUT "$IMMUTABLE_TTL" "$HASH2"
"$TEST_COMMAND" "$ITEM: output looks good" '
test_should_contain "Index of /ipfs/$HASH2" actual
'

test_item "GET IPFS directory file" "http://127.0.0.1:$port/ipfs/$HASH2/test" \
"200 OK" "$DATA2/test" "$IMMUTABLE_TTL"
"200 OK" "$DATA2/test" "$IMMUTABLE_TTL" \
"$(ipfs add --only-hash -q "$DATA2/test")"

ITEM="GET IPFS directory path with index.html"
test_item "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2/has-index-html" \
"302 Found" IGNORE_OUTPUT "$IMMUTABLE_TTL"
"302 Found" IGNORE_OUTPUT "$IMMUTABLE_TTL" \
"$(ipfs add --only-hash -r -q "$DATA2/has-index-html" | tail -n 1)"
"$TEST_COMMAND" "$ITEM: redirects to path/" '
test_should_contain "^Location: /ipfs/$HASH2/has-index-html/.\$" actual.headers
'

test_item "GET IPFS directory path/ with index.html" \
"http://127.0.0.1:$port/ipfs/$HASH2/has-index-html/" \
"200 OK" "$DATA2/has-index-html/index.html" "$IMMUTABLE_TTL"
"200 OK" "$DATA2/has-index-html/index.html" "$IMMUTABLE_TTL" \
"$(ipfs add --only-hash -r -q "$DATA2/has-index-html" | tail -n 1)"

test_item "GET IPFS non-existent file" \
"http://127.0.0.1:$port/ipfs/$HASH2/pleaseDontAddMe" \
"404 Not Found" IGNORE_OUTPUT NO_MAX_AGE
"404 Not Found" IGNORE_OUTPUT NO_MAX_AGE NO_ETAG

# https://github.com/ipfs/go-ipfs/issues/1941 sharness suite: ipfs name
# publish: “Error: failed to find any peer in table”
Expand All @@ -152,7 +171,7 @@ ITEM="GET IPNS path"
ipfs name publish --ttl="${TTL}s" "$HASH1"
'
test_item "$ITEM" "http://127.0.0.1:$port/ipns/$PEERID" \
"200 OK" "$DATA1" "$TTL"
"200 OK" "$DATA1" "$TTL" "$HASH1"
test_timer_start EXPIRY_TIMER "${TTL}s" # The cache entry has expired when this finishes.

ITEM="GET IPNS path again before cache expiry"
Expand All @@ -163,7 +182,7 @@ ITEM="GET IPNS path again before cache expiry"
'
go-sleep 1s # Ensure the following max-age is strictly less than TTL.
test_item "$ITEM" "http://127.0.0.1:$port/ipns/$PEERID" \
"200 OK" "$DATA1" IGNORE_MAX_AGE
"200 OK" "$DATA1" IGNORE_MAX_AGE "$HASH1"
"$TEST_COMMAND" "$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 @@ -183,15 +202,16 @@ test_expect_success "$ITEM: previous version is no longer cached" '

test_item "GET IPNS path again after cache expiry" \
"http://127.0.0.1:$port/ipns/$PEERID/test" \
"200 OK" "$DATA2/test" "$UNKNOWN_TTL"
"200 OK" "$DATA2/test" "$UNKNOWN_TTL" \
"$(ipfs add --only-hash -q "$DATA2/test")"

TEST_COMMAND=test_expect_success

test_item "GET invalid IPFS path" "http://127.0.0.1:$port/ipfs/12345" \
"400 Bad Request" IGNORE_OUTPUT NO_MAX_AGE
"400 Bad Request" IGNORE_OUTPUT NO_MAX_AGE NO_ETAG

test_item "GET invalid root path" "http://127.0.0.1:$port/12345" \
"404 Not Found" IGNORE_OUTPUT NO_MAX_AGE
"404 Not Found" IGNORE_OUTPUT NO_MAX_AGE NO_ETAG

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 6ee526f

Please sign in to comment.