conda-incubator / conda-store-ui

conda-store-ui is a frontend for conda-store powered by react
https://conda-incubator.github.io/conda-store-ui/
BSD 3-Clause "New" or "Revised" License
14 stars 21 forks source link

Ensure artifact base url has a trailing slash #442

Closed soapy1 closed 4 days ago

soapy1 commented 5 days ago

Fixes https://github.com/conda-incubator/conda-store/issues/995

Description

Base url's should have a trailing slash to denote that they are not a relative reference. When there is no trailing slash, using the URL constructor will cause the last part of the path to be cut off.

So, a base url my.server.com/conda-store gets parsed different from my.server.com/conda-store/

ref: https://developer.mozilla.org/en-US/docs/Web/API/URL_API/Resolving_relative_references

So, as an example in src/features/artifacts/components/ArtifactsItem.tsx for a Log artifact

const baseUrl = "http://localhost:8080/conda-store/"
const route = new URL(artifact.route, baseUrl).toString();
// route is correctly `localhost:8080/conda-store/api/v1/build/<build number>/log`

const baseUrl = "http://localhost:8080/conda-store"
const route = new URL(artifact.route, baseUrl).toString();
// route is incorrectly `localhost:8080/api/v1/build/<build number>/log`

Pull request checklist

gabalafou commented 4 days ago

Hey, thanks for putting up a fix for this.

There's an issue that if somebody on the backend later configures the base url to have a trailing slash, then this PR will cause the base url to have two slashes at the end, which may or may not cause a problem for the server, but I would prefer to keep things tidy and not potentially end up with URLs in the future that have double slashes, so I opened a PR that I think fixes the code in a more robust way (#443).

Would you mind giving that one a review while I close this PR?