Skip to content

Simple join of baseUrl and path may result in invalid URLs #1150

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

Closed
1 task done
rdmurphy opened this issue May 26, 2023 · 2 comments
Closed
1 task done

Simple join of baseUrl and path may result in invalid URLs #1150

rdmurphy opened this issue May 26, 2023 · 2 comments
Labels
bug Something isn't working openapi-fetch Relevant to the openapi-fetch library

Comments

@rdmurphy
Copy link

Description

Hello! I noticed that when you have a baseUrl with a trailing slash (e.g. https://example.com/api/) and paths with a leading slash (e.g. /create-post) it results in an invalid URL/request. If I am reading the code correctly I think the problem surfaces in this line:

https://github.com/drwpow/openapi-typescript/blob/65ff4b1826f1a70bce2acf5cd0ba77ca18b08502/packages/openapi-fetch/src/index.ts#L69

I think it assumes those two strings can always just be joined directly. If this is the case then I think the README is also suggesting this would work when it may not.

Reproduction

Here is a tiny types file for making a request to https://jsonplaceholder.typicode.com:

/**
 * This file was auto-generated by openapi-typescript.
 * Do not make direct changes to the file.
 */


export interface paths {
  "/posts": {
    get: {
      responses: {
        /** @description OK */
        200: {
          content: {
            "application/json": ({
                userId?: number;
                id?: number;
                title?: string;
                body?: string;
              })[];
          };
        };
      };
    };
  };
}

And the script I ran:

// I'm using Deno, but it's the same otherwise
import createClient from "https://esm.sh/openapi-fetch@0.2.1";

import type { paths } from "./api.d.ts";

const client = createClient<paths>({
  baseUrl: "https://jsonplaceholder.typicode.com/", // note the trailing slash
});

const posts = await client.get("/posts", {});

Expected result

I think it'd be helpful if openapi-fetch smoothed this over! Maybe the leading slash of a path should be removed pre-joining if the baseUrl has a trailing slash. I could also see this using the URL() constructor instead to get a proper join for free.

Checklist

@rdmurphy rdmurphy added bug Something isn't working openapi-fetch Relevant to the openapi-fetch library labels May 26, 2023
@drwpow
Copy link
Contributor

drwpow commented Jun 16, 2023

Ah you know what? Since OpenAPI requires that all your URLs start with a /, then we could reasonably assume that all requests will, too, and that does mean that baseUrl should have the trailing slash stripped.

If this were any fetch library I’d be hesitant of making assumptions of how URLs are formed (even the URL() constructor allows you to have // in the middle of a pathname), but you’re right we can lean on the OpenAPI spec this enforces.

As a sidenote, we can’t use the URL() constructor method because baseUrl doesn’t have to be a full domain—some people are fetching locally (e.g. { baseUrl: '/api/v1' }).

@drwpow drwpow mentioned this issue Jun 16, 2023
3 tasks
@drwpow
Copy link
Contributor

drwpow commented Jun 16, 2023

This is now fixed in 0.3.0. No stripping of leading slashes in the URL is done, because that should be enforced already by the OpenAPI typing

@drwpow drwpow closed this as completed Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working openapi-fetch Relevant to the openapi-fetch library
Projects
None yet
Development

No branches or pull requests

2 participants