diff --git a/api/swagger.yml b/api/swagger.yml index 5ff14ddc041..1b3082dc85e 100644 --- a/api/swagger.yml +++ b/api/swagger.yml @@ -3910,7 +3910,7 @@ paths: schema: type: string 304: - description: Content Not modified + description: Content not modified 401: $ref: "#/components/responses/Unauthorized" 404: diff --git a/clients/java-legacy/api/openapi.yaml b/clients/java-legacy/api/openapi.yaml index 3b9711a9c90..2e2eca251fb 100644 --- a/clients/java-legacy/api/openapi.yaml +++ b/clients/java-legacy/api/openapi.yaml @@ -4071,7 +4071,7 @@ paths: type: string style: simple "304": - description: Content Not modified + description: Content not modified "401": content: application/json: diff --git a/clients/java-legacy/docs/ObjectsApi.md b/clients/java-legacy/docs/ObjectsApi.md index b412a44ecc5..763634236bc 100644 --- a/clients/java-legacy/docs/ObjectsApi.md +++ b/clients/java-legacy/docs/ObjectsApi.md @@ -411,7 +411,7 @@ Name | Type | Description | Notes **200** | object content | * Content-Length -
* Last-Modified -
* ETag -
| **206** | partial object content | * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
| **302** | Redirect to a pre-signed URL for the object | * Location - redirect to S3
| -**304** | Content Not modified | - | +**304** | Content not modified | - | **401** | Unauthorized | - | **404** | Resource Not Found | - | **410** | object expired | - | diff --git a/clients/java-legacy/src/main/java/io/lakefs/clients/api/ObjectsApi.java b/clients/java-legacy/src/main/java/io/lakefs/clients/api/ObjectsApi.java index 6470a40bb89..1c3ae1d2a52 100644 --- a/clients/java-legacy/src/main/java/io/lakefs/clients/api/ObjectsApi.java +++ b/clients/java-legacy/src/main/java/io/lakefs/clients/api/ObjectsApi.java @@ -564,7 +564,7 @@ public okhttp3.Call deleteObjectsAsync(String repository, String branch, PathLis 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - @@ -662,7 +662,7 @@ private okhttp3.Call getObjectValidateBeforeCall(String repository, String ref, 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - @@ -693,7 +693,7 @@ public File getObject(String repository, String ref, String path, String range, 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - @@ -726,7 +726,7 @@ public ApiResponse getObjectWithHttpInfo(String repository, String ref, St 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - diff --git a/clients/java/api/openapi.yaml b/clients/java/api/openapi.yaml index d04c45908fb..00759b5cb2a 100644 --- a/clients/java/api/openapi.yaml +++ b/clients/java/api/openapi.yaml @@ -4071,7 +4071,7 @@ paths: type: string style: simple "304": - description: Content Not modified + description: Content not modified "401": content: application/json: diff --git a/clients/java/docs/ObjectsApi.md b/clients/java/docs/ObjectsApi.md index 0481e83e995..d441235e74e 100644 --- a/clients/java/docs/ObjectsApi.md +++ b/clients/java/docs/ObjectsApi.md @@ -420,7 +420,7 @@ public class Example { | **200** | object content | * Content-Length -
* Last-Modified -
* ETag -
| | **206** | partial object content | * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
| | **302** | Redirect to a pre-signed URL for the object | * Location - redirect to S3
| -| **304** | Content Not modified | - | +| **304** | Content not modified | - | | **401** | Unauthorized | - | | **404** | Resource Not Found | - | | **410** | object expired | - | diff --git a/clients/java/src/main/java/io/lakefs/clients/sdk/ObjectsApi.java b/clients/java/src/main/java/io/lakefs/clients/sdk/ObjectsApi.java index 9bc1f58b9cc..8f2ea7cc92d 100644 --- a/clients/java/src/main/java/io/lakefs/clients/sdk/ObjectsApi.java +++ b/clients/java/src/main/java/io/lakefs/clients/sdk/ObjectsApi.java @@ -864,7 +864,7 @@ public APIgetObjectRequest presign(Boolean presign) { 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - @@ -887,7 +887,7 @@ public okhttp3.Call buildCall(final ApiCallback _callback) throws ApiException { 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - @@ -911,7 +911,7 @@ public File execute() throws ApiException { 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - @@ -935,7 +935,7 @@ public ApiResponse executeWithHttpInfo() throws ApiException { 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - @@ -962,7 +962,7 @@ public okhttp3.Call executeAsync(final ApiCallback _callback) throws ApiEx 200 object content * Content-Length -
* Last-Modified -
* ETag -
206 partial object content * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
302 Redirect to a pre-signed URL for the object * Location - redirect to S3
- 304 Content Not modified - + 304 Content not modified - 401 Unauthorized - 404 Resource Not Found - 410 object expired - diff --git a/clients/python-legacy/docs/ObjectsApi.md b/clients/python-legacy/docs/ObjectsApi.md index 01032ddd654..e84269d94c9 100644 --- a/clients/python-legacy/docs/ObjectsApi.md +++ b/clients/python-legacy/docs/ObjectsApi.md @@ -507,7 +507,7 @@ Name | Type | Description | Notes **200** | object content | * Content-Length -
* Last-Modified -
* ETag -
| **206** | partial object content | * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
| **302** | Redirect to a pre-signed URL for the object | * Location - redirect to S3
| -**304** | Content Not modified | - | +**304** | Content not modified | - | **401** | Unauthorized | - | **404** | Resource Not Found | - | **410** | object expired | - | diff --git a/clients/python/docs/ObjectsApi.md b/clients/python/docs/ObjectsApi.md index fa7af142ea9..a9ab5b4e74a 100644 --- a/clients/python/docs/ObjectsApi.md +++ b/clients/python/docs/ObjectsApi.md @@ -480,7 +480,7 @@ Name | Type | Description | Notes **200** | object content | * Content-Length -
* Last-Modified -
* ETag -
| **206** | partial object content | * Content-Length -
* Content-Range -
* Last-Modified -
* ETag -
| **302** | Redirect to a pre-signed URL for the object | * Location - redirect to S3
| -**304** | Content Not modified | - | +**304** | Content not modified | - | **401** | Unauthorized | - | **404** | Resource Not Found | - | **410** | object expired | - | diff --git a/docs/assets/js/swagger.yml b/docs/assets/js/swagger.yml index 5ff14ddc041..1b3082dc85e 100644 --- a/docs/assets/js/swagger.yml +++ b/docs/assets/js/swagger.yml @@ -3910,7 +3910,7 @@ paths: schema: type: string 304: - description: Content Not modified + description: Content not modified 401: $ref: "#/components/responses/Unauthorized" 404: diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 4ab2aaa9317..974a4108e6d 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -4197,10 +4197,11 @@ func (c *Controller) GetObject(w http.ResponseWriter, r *http.Request, repositor return } - etag := httputil.ETag(entry.Checksum) + eTag := httputil.ETag(entry.Checksum) // check ETag if not modified in request - if swag.StringValue(params.IfNoneMatch) == etag { + // Not using swag.StringValue to catch edge case where eTag == "" + if params.IfNoneMatch != nil && *params.IfNoneMatch == eTag { w.WriteHeader(http.StatusNotModified) return } @@ -4222,7 +4223,7 @@ func (c *Controller) GetObject(w http.ResponseWriter, r *http.Request, repositor } // set response headers - w.Header().Set("ETag", etag) + w.Header().Set("ETag", eTag) lastModified := httputil.HeaderTimestamp(entry.CreationDate) w.Header().Set("Last-Modified", lastModified) w.Header().Set("Content-Type", entry.ContentType) diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index ce56d8eb20f..0c4a5e3e877 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -45,10 +45,7 @@ import ( "golang.org/x/exp/slices" ) -const ( - DefaultUserID = "example_user" - DefaultETag = `"3c4838fe975c762ee97cf39fbbe566f1"` -) +const DefaultUserID = "example_user" type Statuser interface { StatusCode() int @@ -2504,13 +2501,14 @@ func TestController_ObjectsHeadObjectHandler(t *testing.T) { Expired: true, } testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, "main", expired)) + expectedEtag := "\"" + blob.Checksum + "\"" t.Run("head object", func(t *testing.T) { resp, err := clt.HeadObjectWithResponse(ctx, repo, "main", &apigen.HeadObjectParams{Path: "foo/bar"}) require.Nil(t, err) require.Equal(t, http.StatusOK, resp.HTTPResponse.StatusCode) require.Equal(t, int64(37), resp.HTTPResponse.ContentLength) - require.Equal(t, DefaultETag, resp.HTTPResponse.Header.Get("ETag")) + require.Equal(t, expectedEtag, resp.HTTPResponse.Header.Get("ETag")) require.Empty(t, string(resp.Body)) }) @@ -2523,7 +2521,7 @@ func TestController_ObjectsHeadObjectHandler(t *testing.T) { require.Nil(t, err) require.Equal(t, http.StatusPartialContent, resp.HTTPResponse.StatusCode) require.Equal(t, int64(10), resp.HTTPResponse.ContentLength) - require.Equal(t, DefaultETag, resp.HTTPResponse.Header.Get("ETag")) + require.Equal(t, expectedEtag, resp.HTTPResponse.Header.Get("ETag")) require.Empty(t, string(resp.Body)) }) @@ -2570,15 +2568,16 @@ func TestController_ObjectsGetObjectHandler(t *testing.T) { } testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, "main", entry)) - expired := catalog.DBEntry{ - Path: "foo/expired", - PhysicalAddress: "an_expired_physical_address", + emptyEtag := catalog.DBEntry{ + Path: "foo/empty-etag", + PhysicalAddress: blob.PhysicalAddress, CreationDate: time.Now(), - Size: 99999, - Checksum: "b10b", + Size: blob.Size, + Checksum: "", Expired: true, } - testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, "main", expired)) + testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, "main", emptyEtag)) + expectedEtag := "\"" + blob.Checksum + "\"" t.Run("get object", func(t *testing.T) { resp, err := clt.GetObjectWithResponse(ctx, repo, "main", &apigen.GetObjectParams{Path: "foo/bar"}) @@ -2593,9 +2592,7 @@ func TestController_ObjectsGetObjectHandler(t *testing.T) { t.Errorf("expected 37 bytes in content length, got back %d", resp.HTTPResponse.ContentLength) } etag := resp.HTTPResponse.Header.Get("ETag") - if etag != DefaultETag { - t.Errorf("got unexpected etag: %s", etag) - } + require.Equal(t, etag, expectedEtag) body := string(resp.Body) if body != "this is file content made up of bytes" { @@ -2621,9 +2618,7 @@ func TestController_ObjectsGetObjectHandler(t *testing.T) { } etag := resp.HTTPResponse.Header.Get("ETag") - if etag != DefaultETag { - t.Errorf("got unexpected etag: %s", etag) - } + require.Equal(t, etag, expectedEtag) body := string(resp.Body) if body != "this is fi" { @@ -2661,8 +2656,7 @@ func TestController_ObjectsGetObjectHandler(t *testing.T) { }) t.Run("get object returns expected response with different etag", func(t *testing.T) { - newChecksum := `"11ee22ff33445566778899"` - eTagInput := "\"" + newChecksum + "\"" + eTagInput := "\"11ee22ff33445566778899\"" resp, err := clt.GetObjectWithResponse(ctx, repo, "main", &apigen.GetObjectParams{ Path: "foo/bar", IfNoneMatch: &eTagInput, @@ -2670,13 +2664,33 @@ func TestController_ObjectsGetObjectHandler(t *testing.T) { require.Nil(t, err) require.Equal(t, http.StatusOK, resp.HTTPResponse.StatusCode) require.Equal(t, int64(37), resp.HTTPResponse.ContentLength) - require.Equal(t, DefaultETag, resp.HTTPResponse.Header.Get("ETag")) + require.Equal(t, expectedEtag, resp.HTTPResponse.Header.Get("ETag")) }) t.Run("get object returns not modified with same etag", func(t *testing.T) { - eTagInput := "\"" + blob.Checksum + "\"" resp, err := clt.GetObjectWithResponse(ctx, repo, "main", &apigen.GetObjectParams{ Path: "foo/bar", + IfNoneMatch: &expectedEtag, + }) + require.Nil(t, err) + require.Equal(t, http.StatusNotModified, resp.HTTPResponse.StatusCode) + require.Equal(t, int64(0), resp.HTTPResponse.ContentLength) + require.Empty(t, resp.HTTPResponse.Header.Get("ETag")) + }) + + t.Run("get object returns expected response for empty etag", func(t *testing.T) { + resp, err := clt.GetObjectWithResponse(ctx, repo, "main", &apigen.GetObjectParams{ + Path: "foo/empty-etag", + }) + require.Nil(t, err) + require.Equal(t, http.StatusOK, resp.HTTPResponse.StatusCode) + require.Equal(t, int64(37), resp.HTTPResponse.ContentLength) + require.Equal(t, "\"\"", resp.HTTPResponse.Header.Get("ETag")) + }) + t.Run("get object with if-none-match returns expected response for empty etag", func(t *testing.T) { + eTagInput := "\"\"" + resp, err := clt.GetObjectWithResponse(ctx, repo, "main", &apigen.GetObjectParams{ + Path: "foo/empty-etag", IfNoneMatch: &eTagInput, }) require.Nil(t, err)