Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Last-Modified header in response is not formatted correctly #271

Open
ricardo-kagawa opened this issue Mar 6, 2024 · 2 comments
Open

Last-Modified header in response is not formatted correctly #271

ricardo-kagawa opened this issue Mar 6, 2024 · 2 comments
Labels
SDK Issue pertains to the SDK itself and not specific to any service

Comments

@ricardo-kagawa
Copy link

At least for the lastModified field of getObject responses from ObjectStorageClient, and likely for any client generating response fields of type Date from HTTP headers, the response field is formatting the parsed header value using the local time zone, despite the formatted value using the Z (UTC) time zone specifier.

export function formatDateToRFC3339(date: Date): string {

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/getHours

The getHours() method of Date instances returns the hours for this date according to local time.

Also, the SDK documentation mentions it should have the Date type, but the field is of type string. The TypeScript declaration also declares it as Date, but the value seems to come verbatim from that formatter, which declares a string return value as seen above.

The following code should reproduce these issues in Deno:

// deno test --allow-all repro.ts
import type { HttpClient, HttpRequest } from "npm:oci-common";

import "npm:bunyan";
import { ObjectStorageClient } from "npm:oci-objectstorage";

import { assertEquals } from "https://deno.land/[email protected]/assert/mod.ts";

class TestHttpClient implements HttpClient {
  send(_req: HttpRequest): Promise<Response> {
    return Promise.resolve(
      new Response("test-data", {
        headers: { "last-modified": "Tue, 15 Nov 1994 12:45:26 GMT" },
      }),
    );
  }
}

Deno.test("lastModified in getObject response", async () => {
  const httpClient = new TestHttpClient();
  const client = new ObjectStorageClient({ httpClient });

  try {
    const resp = await client.getObject({
      bucketName: "test-bucket",
      objectName: "test-object",
      namespaceName: "test-namespace",
    });

    // The field is declared as Date, but it is actually a string
    const lastModified = (resp.lastModified as unknown) as string;
    assertEquals(lastModified, "1994-11-15T12:45:26Z");
  } finally {
    client.shutdownCircuitBreaker();
  }
});
@ricardo-kagawa
Copy link
Author

Forgot to mention that the issue can be worked around using the UTC time zone, like this (in Linux):

TZ=UTC deno test --allow-all repro.ts

@bhagwatvyas bhagwatvyas added the SDK Issue pertains to the SDK itself and not specific to any service label Mar 21, 2024
@JoshuaWR
Copy link
Member

Hi @ricardo-kagawa, thanks for highlighting this issue. I think you're right on both accounts. Looks like we should be using getUTCHours() instead of getHours() (as well as minutes, seconds, etc.). Additionally we should be returning a Date from the formatter, as it looks like the other SDKs return some sort of Date/Time object, and not a string. This should be a relatively easy, I will work with the team to implement it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SDK Issue pertains to the SDK itself and not specific to any service
Projects
None yet
Development

No branches or pull requests

3 participants