oracle/oci-typescript-sdk

Last-Modified header in response is not formatted correctly

Opened this issue · 2 comments

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/std@0.216.0/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();
  }
});

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

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.