oracle / oci-typescript-sdk

Oracle Cloud Infrastructure SDK for TypeScript and JavaScript
https://docs.oracle.com/en-us/iaas/Content/API/SDKDocs/typescriptsdk.htm
Other
68 stars 50 forks source link

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

Open ricardo-kagawa opened 5 months ago

ricardo-kagawa commented 5 months ago

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.

https://github.com/oracle/oci-typescript-sdk/blob/52a6526faa186dfdc3f8af6f52458ac29e4dc458/lib/common/lib/helper.ts#L299

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();
  }
});
ricardo-kagawa commented 5 months ago

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
JoshuaWR commented 4 months ago

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.